[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] linux-user: improved fake /proc/self/stat
From: |
Fabio Erculiani |
Subject: |
Re: [Qemu-devel] linux-user: improved fake /proc/self/stat |
Date: |
Tue, 3 Jan 2012 16:31:27 +0100 |
On Tue, Jan 3, 2012 at 3:26 PM, Alexander Graf <address@hidden> 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