| Message ID | CAN3AtvrWf4bRmzDJzkUZo+2rkEb6ZnKKGGdD4QR2q5bqFvT05A@mail.gmail.com |
|---|---|
| State | New |
| Headers | show |
Subject should have [PATCH] in the beginning. Why don't you just create the patch using git-format-patch? It makes sure the format is correct. You can also check out http://wiki.qemu.org/Contribute/SubmitAPatch for some hints. And thanks a lot for fixing this! On 03.01.2012, at 08:25, Fabio Erculiani wrote: > Hi all, > this is a patch on top of "[PATCH 4/5] linux-user: fake > /proc/self/stat" (sorry I couldn't find the git repo bound to this > patchwork) that also implements PID and binary name reading them from > the fake TSS. > The pid was just a "why not" while the binary name is required (as > well as the backets in order to make `ps` not segfault, because it > expects to find "(binary name)" on the second spot. > > The only doubt I have is about printing ts->ts_tid which is a pid_t, > which is a signed int, so perhaps int would be enough. Signed-off-by line is missing :). > > --- qemu-1.0.orig/linux-user/syscall.c > +++ qemu-1.0/linux-user/syscall.c > @@ -4678,11 +4678,23 @@ static int open_self_stat(void *cpu_env, > int len; > uint64_t val = 0; > > - if (i == 27) { > + if (i == 0) { > + /* pid */ > + snprintf(buf, sizeof(buf), "%jd ", (intmax_t) ts->ts_tid); Is it the TID or the PID? If it's get PID, this should be getpid(). Also why not simply use uint64_t and PRId64 like below? > + } > + else if (i == 1) { Braces are incorrect. Please run scripts/checkpatch.pl on your patch :). > + /* app name */ > + snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->filename); I would assume this is argv[0] and not the filename, so if I read the code correctly this should be ts->bprm->>argv[0], right? > + } > + else if (i == 27) { Braces again :) > /* stack bottom */ > val = start_stack; > + snprintf(buf, sizeof(buf), "%"PRId64 " ", val); > + } > + else { Braces Alex > + /* for the rest, write zeros */ > + snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' '); > } > - snprintf(buf, sizeof(buf), "%"PRId64 "%c", val, i == 43 ? '\n' : ' '); > len = strlen(buf); > if (write(fd, buf, len) != len) { > return -1; > > -- > Fabio Erculiani
On Tue, Jan 3, 2012 at 3:26 PM, Alexander Graf <agraf@suse.de> wrote: > Subject should have [PATCH] in the beginning. Why don't you just create the patch using git-format-patch? It makes sure the format is correct. You can also check out http://wiki.qemu.org/Contribute/SubmitAPatch for some hints. > > And thanks a lot for fixing this! whoops, yeah this is my first contribution to qemu ;) > > On 03.01.2012, at 08:25, Fabio Erculiani wrote: > >> Hi all, >> this is a patch on top of "[PATCH 4/5] linux-user: fake >> /proc/self/stat" (sorry I couldn't find the git repo bound to this >> patchwork) that also implements PID and binary name reading them from >> the fake TSS. >> The pid was just a "why not" while the binary name is required (as >> well as the backets in order to make `ps` not segfault, because it >> expects to find "(binary name)" on the second spot. >> >> The only doubt I have is about printing ts->ts_tid which is a pid_t, >> which is a signed int, so perhaps int would be enough. > > Signed-off-by line is missing :). > >> >> --- qemu-1.0.orig/linux-user/syscall.c >> +++ qemu-1.0/linux-user/syscall.c >> @@ -4678,11 +4678,23 @@ static int open_self_stat(void *cpu_env, >> int len; >> uint64_t val = 0; >> >> - if (i == 27) { >> + if (i == 0) { >> + /* pid */ >> + snprintf(buf, sizeof(buf), "%jd ", (intmax_t) ts->ts_tid); > > Is it the TID or the PID? If it's get PID, this should be getpid(). Also why not simply use uint64_t and PRId64 like below? ts_tid contains the pid afaik. But ok, will use getpid() directly. > >> + } >> + else if (i == 1) { > > Braces are incorrect. Please run scripts/checkpatch.pl on your patch :). Ok, thanks, will validate using checkpatch.pl. > >> + /* app name */ >> + snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->filename); > > I would assume this is argv[0] and not the filename, so if I read the code correctly this should be ts->bprm->>argv[0], right? whoops :) Yes. > >> + } >> + else if (i == 27) { > > Braces again :) > >> /* stack bottom */ >> val = start_stack; >> + snprintf(buf, sizeof(buf), "%"PRId64 " ", val); >> + } >> + else { > > Braces > > > Alex > >> + /* for the rest, write zeros */ >> + snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' '); >> } >> - snprintf(buf, sizeof(buf), "%"PRId64 "%c", val, i == 43 ? '\n' : ' '); >> len = strlen(buf); >> if (write(fd, buf, len) != len) { >> return -1; >> >> -- >> Fabio Erculiani >
--- qemu-1.0.orig/linux-user/syscall.c +++ qemu-1.0/linux-user/syscall.c @@ -4678,11 +4678,23 @@ static int open_self_stat(void *cpu_env, int len; uint64_t val = 0; - if (i == 27) { + if (i == 0) { + /* pid */ + snprintf(buf, sizeof(buf), "%jd ", (intmax_t) ts->ts_tid); + } + else if (i == 1) { + /* app name */ + snprintf(buf, sizeof(buf), "(%s) ", ts->bprm->filename); + } + else if (i == 27) { /* stack bottom */ val = start_stack; + snprintf(buf, sizeof(buf), "%"PRId64 " ", val); + } + else { + /* for the rest, write zeros */ + snprintf(buf, sizeof(buf), "0%c", i == 43 ? '\n' : ' '); } - snprintf(buf, sizeof(buf), "%"PRId64 "%c", val, i == 43 ? '\n' : ' '); len = strlen(buf); if (write(fd, buf, len) != len) { return -1;