[PATCH v1] Initialize local variables in functions

[Zubin Mithra]

Hi Mike,

On Wed, Aug 6, 2014 at 6:59 PM, Mike Frysinger <vapier at gentoo.org> wrote:
> On Mon 04 Aug 2014 08:35:02 zubin.mithra at gmail.com wrote:
>> From: Zubin Mithra <zubin.mithra at gmail.com>
>>
>> * desc.c (decode_select): Initialize fds to NULL.
>> * strace.c (expand_tcbtab): Change type to unsigned.
>> (startup_child): Initialize pathname array.
>
> you need to describe why you're making a change, not just how

Indeed, I'll make sure to add in descriptions from now on.

>
>> --- a/desc.c
>> +++ b/desc.c
>> @@ -314,7 +314,7 @@ decode_select(struct tcb *tcp, long *args, enum
>> bitness_t bitness) {
>>       int i, j;
>>       int nfds, fdsize;
>> -     fd_set *fds;
>> +     fd_set *fds = NULL;
>>       const char *sep;
>>       long arg;
>
> why ?  i guess you're handling the case where nfds==0 ?  seems like fds is
> used uninitialized in that case.
>
> however, if you do this, you'll see that the nfds<0 check also sets fds to
> NULL, so you should delete that line.

I was wondering if there could be a situation where nfds > 0 and the
calculated fdsize would end up equal to 0. In such a case fds would be
uninitialised.

>
>> --- a/strace.c
>> +++ b/strace.c
>> @@ -675,7 +675,7 @@ expand_tcbtab(void)
>>          callers have pointers and it would be a pain.
>>          So tcbtab is a table of pointers.  Since we never
>>          free the TCBs, we allocate a single chunk of many.  */
>> -     int i = tcbtabsize;
>> +     unsigned int i = tcbtabsize;
>
> i guess this is because tcpbtabsize is unsigned, and i is only used to compare
> to that.

Indeed, its used to compare against the size so there didn't seem any
reason to have it as a signed integer.

>
>> @@ -1160,7 +1160,7 @@ startup_child(char **argv)
>>  {
>>       struct_stat statbuf;
>>       const char *filename;
>> -     char pathname[MAXPATHLEN];
>> +     char pathname[MAXPATHLEN] = "";
>>       int pid;
>>       struct tcb *tcp;
>
> i'm not seeing a problem here.  pathname looks like it's always initialized
> before it's used.  note that sizeof(pathname) is not using the value of
> pathname, so those checks don't count ...

Ah indeed, I misunderstood(for some reason I was thinking of strcat
instead of strcpy). This shouldn't be a problem.

Thanks,
-- zm