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

[Fabio Erculiani <lxnay@sabayon.org>]

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