On 10/10/2017 01:48 PM, Corinna Vinschen wrote:
> Hi Michael,
> 
> On Oct  9 18:58, Michael Haubenwallner wrote:
>> When fork fails, we can use "%s" now with system_sprintf for the errmsg
>> rather than a (potentially too small) buffer for the format string.
> 
> How could buf be too small?

See below.

Actually I've found this by searching for suspect char array definitions
while hunting the "uninitialized variable for RtlLookupFunctionEntry" bug.

>> * fork.cc (fork): Use "%s" with system_printf now.
>> ---
>>  winsup/cygwin/fork.cc | 9 ++-------
>>  1 file changed, 2 insertions(+), 7 deletions(-)
>>
>> diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
>> index 73a72f530..bcbef12d8 100644
>> --- a/winsup/cygwin/fork.cc
>> +++ b/winsup/cygwin/fork.cc
>> @@ -618,13 +618,8 @@ fork ()
>>        if (!grouped.errmsg)
>>      syscall_printf ("fork failed - child pid %d, errno %d", 
>> grouped.child_pid, grouped.this_errno);
>>        else
>> -    {
>> -      char buf[strlen (grouped.errmsg) + sizeof ("child %d - , errno 
>> 4294967295  ")];

Usually child_pid is longer than the 2 characters counted by "%d", but
errno usually is shorther than the 10 characters counted by "4294967295",
and there is another 2 reserved characters counted by trailing "  ".

In practice the buffer unlikely will be too small, so this is merely cosmetics.

>> -      strcpy (buf, "child %d - ");
>> -      strcat (buf, grouped.errmsg);
>> -      strcat (buf, ", errno %d");
>> -      system_printf (buf, grouped.child_pid, grouped.this_errno);
>> -    }
>> +    system_printf ("child %d - %s, errno %d", grouped.child_pid,
>> +                   grouped.errmsg, grouped.this_errno);
>>  
>>        set_errno (grouped.this_errno);
>>      }
>> -- 
>> 2.14.2
> 
> I guess this also means we can drop the if/else, kind of like
> 
>   system_printf ("child %d %s%s, errno %d",
>                grouped.child_pid,
>                grouped.errmsg ? "- " : "",
>                grouped.errmsg ?: "",
>                grouped.this_errno);
> 
> What do you think?

Nothing I really take care of - yet suggesting:

  system_printf ("fork failed - child %d%s%s, errno %d",
                 grouped.child_pid,
                 grouped.errmsg ? " - " : "",
                 grouped.errmsg ?: "",
                 grouped.this_errno);

But wait, what's the difference between syscall_printf and system_printf?

/haubi/

Reply via email to