[PATCH 0/2] Fix uninitialized memory read and comment typo

[PATCH 0/2] Fix uninitialized memory read and comment typo

[Pat Notz]

One-liner corrections.

Pat Notz (2):
  dir.c: fix uninitialized memory warning
  strbuf.h: fix comment typo

 dir.c    |    2 +-
 strbuf.h |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
1.7.2.3

[PATCH 1/2] dir.c: fix uninitialized memory warning

[Pat Notz]

GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 dir.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/dir.c b/dir.c
index 133f472..d1e5e5e 100644
--- a/dir.c
+++ b/dir.c
@@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,
 {
 	struct stat st;
 	int fd, i;
-	size_t size;
+	size_t size = 0;
 	char *buf, *entry;
 
 	fd = open(fname, O_RDONLY);
-- 
1.7.2.3

[PATCH 2/2] strbuf.h: fix comment typo

[Pat Notz]

Signed-off-by: Pat Notz <patnotz@gmail.com>
---
 strbuf.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/strbuf.h b/strbuf.h
index fac2dbc..675a91f 100644
--- a/strbuf.h
+++ b/strbuf.h
@@ -16,7 +16,7 @@
  *
  * 2. the ->buf member is a byte array that has at least ->len + 1 bytes
  *    allocated. The extra byte is used to store a '\0', allowing the ->buf
- *    member to be a valid C-string. Every strbuf function ensure this
+ *    member to be a valid C-string. Every strbuf function ensures this
  *    invariant is preserved.
  *
  *    Note that it is OK to "play" with the buffer directly if you work it
-- 
1.7.2.3

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Ævar Arnfjörð Bjarmason]

On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:
> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.
>
> Signed-off-by: Pat Notz <patnotz@gmail.com>
> ---
>  dir.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/dir.c b/dir.c
> index 133f472..d1e5e5e 100644
> --- a/dir.c
> +++ b/dir.c
> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,
>  {
>        struct stat st;
>        int fd, i;
> -       size_t size;
> +       size_t size = 0;
>        char *buf, *entry;

What does the GCC warning say exactl? I.e. what line does it complain
about?

Maybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't
checked.

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Nguyen Thai Ngoc Duy]

2010/9/17 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:
> On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:
>> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.
>>
>> Signed-off-by: Pat Notz <patnotz@gmail.com>
>> ---
>>  dir.c |    2 +-
>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>
>> diff --git a/dir.c b/dir.c
>> index 133f472..d1e5e5e 100644
>> --- a/dir.c
>> +++ b/dir.c
>> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,
>>  {
>>        struct stat st;
>>        int fd, i;
>> -       size_t size;
>> +       size_t size = 0;
>>        char *buf, *entry;
>
> What does the GCC warning say exactl? I.e. what line does it complain
> about?
>
> Maybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't
> checked.

I don't see any case that "size" can be used uninitialized. Maybe the
compiler was confused by

if (!check_index ||
    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
        return -1;

I wouldn't hurt though to initialize it early, even just to stop the
compiler from complaining.
-- 
Duy

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Pat Notz]

On Thu, Sep 16, 2010 at 5:26 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> 2010/9/17 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:
>> On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:
>>> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.
>>>
>>> Signed-off-by: Pat Notz <patnotz@gmail.com>
>>> ---
>>>  dir.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/dir.c b/dir.c
>>> index 133f472..d1e5e5e 100644
>>> --- a/dir.c
>>> +++ b/dir.c
>>> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,
>>>  {
>>>        struct stat st;
>>>        int fd, i;
>>> -       size_t size;
>>> +       size_t size = 0;
>>>        char *buf, *entry;
>>
>> What does the GCC warning say exactl? I.e. what line does it complain
>> about?

Here's the output:

make V=1 -j2 all
gcc -o dir.o -c   -g -O2 -Wall -I. -I/opt/local/include
-DUSE_ST_TIMESPEC  -DSHA1_HEADER='<openssl/sha.h>'  -DNO_MEMMEM  dir.c
dir.c: In function 'add_excludes_from_file_to_list':
dir.c:235: warning: 'size' may be used uninitialized in this function


>>
>> Maybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't
>> checked.
>
> I don't see any case that "size" can be used uninitialized. Maybe the
> compiler was confused by
>
> if (!check_index ||
>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
>        return -1;
>

No, line 245: if(size==0)

> I wouldn't hurt though to initialize it early, even just to stop the
> compiler from complaining.
> --
> Duy
>

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Nguyen Thai Ngoc Duy]

On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:
>> I don't see any case that "size" can be used uninitialized. Maybe the
>> compiler was confused by
>>
>> if (!check_index ||
>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
>>        return -1;
>>
>
> No, line 245: if(size==0)

The only chance for that line to be executed is read_skip_*() is
executed and returns non-NULL buf. read_skip*() returns a non-NULL
buffer at the end of function and does set size right before
returning.

To me it looks like a false alarm. But again, no objection to the patch.
-- 
Duy

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Pat Notz]

On Thu, Sep 16, 2010 at 7:04 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
> On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:
>>> I don't see any case that "size" can be used uninitialized. Maybe the
>>> compiler was confused by
>>>
>>> if (!check_index ||
>>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
>>>        return -1;
>>>
>>
>> No, line 245: if(size==0)
>
> The only chance for that line to be executed is read_skip_*() is
> executed and returns non-NULL buf. read_skip*() returns a non-NULL
> buffer at the end of function and does set size right before
> returning.
>
> To me it looks like a false alarm. But again, no objection to the patch.

I agree that it's a false alarm which is why I wasn't too interested
in looking into it very deeply.  Just looking to keep the code warning
free is all.

> --
> Duy
>

Re: [PATCH 1/2] dir.c: fix uninitialized memory warning

[Pat Notz]

For anyone who care, this warning was actually emitted by the version
of GCC that ships with MacOS 10.5: i686-apple-darwin9-gcc-4.0.1 (GCC)
4.0.1 (Apple Inc. build 5493).

GCC 4.4.4 does *not* git this warning.

Sorry for the confusion, my IDE was using a different $PATH than my shell.


On Thu, Sep 16, 2010 at 7:13 PM, Pat Notz <patnotz@gmail.com> wrote:
> On Thu, Sep 16, 2010 at 7:04 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:
>> On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:
>>>> I don't see any case that "size" can be used uninitialized. Maybe the
>>>> compiler was confused by
>>>>
>>>> if (!check_index ||
>>>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)
>>>>        return -1;
>>>>
>>>
>>> No, line 245: if(size==0)
>>
>> The only chance for that line to be executed is read_skip_*() is
>> executed and returns non-NULL buf. read_skip*() returns a non-NULL
>> buffer at the end of function and does set size right before
>> returning.
>>
>> To me it looks like a false alarm. But again, no objection to the patch.
>
> I agree that it's a false alarm which is why I wasn't too interested
> in looking into it very deeply.  Just looking to keep the code warning
> free is all.
>
>> --
>> Duy
>>
>