[Qemu-devel] linux-user: improved fake /proc/self/stat

[Qemu-devel] linux-user: improved fake /proc/self/stat

[Fabio Erculiani]

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.

--- 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;

--
Fabio Erculiani

Re: [Qemu-devel] linux-user: improved fake /proc/self/stat

[Alexander Graf]

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

Re: [Qemu-devel] linux-user: improved fake /proc/self/stat

[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
>



-- 
Fabio Erculiani