{"thread":{"id":"25124","subject":"[PATCH 0/2] Fix uninitialized memory read and comment typo","startedAt":"2010-09-16T20:53:21Z","lastAt":"2010-09-17T17:23:13Z","messageCount":9,"participants":["Pat Notz","Ævar Arnfjörð Bjarmason","Nguyen Thai Ngoc Duy"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"150846","messageId":"1284670403-90716-1-git-send-email-patnotz@gmail.com","threadId":"25124","inReplyTo":null,"subject":"[PATCH 0/2] Fix uninitialized memory read and comment typo","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-16T20:53:21Z","receivedAt":"2010-09-16T20:53:21Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"One-liner corrections.\n\nPat Notz (2):\n  dir.c: fix uninitialized memory warning\n  strbuf.h: fix comment typo\n\n dir.c    |    2 +-\n strbuf.h |    2 +-\n 2 files changed, 2 insertions(+), 2 deletions(-)\n\n-- \n1.7.2.3\n"},{"id":"150847","messageId":"1284670403-90716-2-git-send-email-patnotz@gmail.com","threadId":"25124","inReplyTo":"1284670403-90716-1-git-send-email-patnotz@gmail.com","subject":"[PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-16T20:53:22Z","receivedAt":"2010-09-16T20:53:22Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.\n\nSigned-off-by: Pat Notz <patnotz@gmail.com>\n---\n dir.c |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/dir.c b/dir.c\nindex 133f472..d1e5e5e 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,\n {\n \tstruct stat st;\n \tint fd, i;\n-\tsize_t size;\n+\tsize_t size = 0;\n \tchar *buf, *entry;\n \n \tfd = open(fname, O_RDONLY);\n-- \n1.7.2.3\n"},{"id":"150848","messageId":"1284670403-90716-3-git-send-email-patnotz@gmail.com","threadId":"25124","inReplyTo":"1284670403-90716-1-git-send-email-patnotz@gmail.com","subject":"[PATCH 2/2] strbuf.h: fix comment typo","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-16T20:53:23Z","receivedAt":"2010-09-16T20:53:23Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"Signed-off-by: Pat Notz <patnotz@gmail.com>\n---\n strbuf.h |    2 +-\n 1 files changed, 1 insertions(+), 1 deletions(-)\n\ndiff --git a/strbuf.h b/strbuf.h\nindex fac2dbc..675a91f 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -16,7 +16,7 @@\n  *\n  * 2. the ->buf member is a byte array that has at least ->len + 1 bytes\n  *    allocated. The extra byte is used to store a '\\0', allowing the ->buf\n- *    member to be a valid C-string. Every strbuf function ensure this\n+ *    member to be a valid C-string. Every strbuf function ensures this\n  *    invariant is preserved.\n  *\n  *    Note that it is OK to \"play\" with the buffer directly if you work it\n-- \n1.7.2.3\n"},{"id":"150856","messageId":"AANLkTim4SiuX=aWLeYXKpgvD+Nh1trH8qgf3V36iVa9w@mail.gmail.com","threadId":"25124","inReplyTo":"1284670403-90716-2-git-send-email-patnotz@gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2010-09-16T23:13:11Z","receivedAt":"2010-09-16T23:13:11Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:\n> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.\n>\n> Signed-off-by: Pat Notz <patnotz@gmail.com>\n> ---\n>  dir.c |    2 +-\n>  1 files changed, 1 insertions(+), 1 deletions(-)\n>\n> diff --git a/dir.c b/dir.c\n> index 133f472..d1e5e5e 100644\n> --- a/dir.c\n> +++ b/dir.c\n> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>  {\n>        struct stat st;\n>        int fd, i;\n> -       size_t size;\n> +       size_t size = 0;\n>        char *buf, *entry;\n\nWhat does the GCC warning say exactl? I.e. what line does it complain\nabout?\n\nMaybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't\nchecked.\n"},{"id":"150857","messageId":"AANLkTik1X0i-OYZCxokw-W3Kt+vEDtBvFeCwQU3q40ap@mail.gmail.com","threadId":"25124","inReplyTo":"AANLkTim4SiuX=aWLeYXKpgvD+Nh1trH8qgf3V36iVa9w@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-16T23:26:39Z","receivedAt":"2010-09-16T23:26:39Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"2010/9/17 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:\n> On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:\n>> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.\n>>\n>> Signed-off-by: Pat Notz <patnotz@gmail.com>\n>> ---\n>>  dir.c |    2 +-\n>>  1 files changed, 1 insertions(+), 1 deletions(-)\n>>\n>> diff --git a/dir.c b/dir.c\n>> index 133f472..d1e5e5e 100644\n>> --- a/dir.c\n>> +++ b/dir.c\n>> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>>  {\n>>        struct stat st;\n>>        int fd, i;\n>> -       size_t size;\n>> +       size_t size = 0;\n>>        char *buf, *entry;\n>\n> What does the GCC warning say exactl? I.e. what line does it complain\n> about?\n>\n> Maybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't\n> checked.\n\nI don't see any case that \"size\" can be used uninitialized. Maybe the\ncompiler was confused by\n\nif (!check_index ||\n    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)\n        return -1;\n\nI wouldn't hurt though to initialize it early, even just to stop the\ncompiler from complaining.\n-- \nDuy\n"},{"id":"150858","messageId":"AANLkTin52McRcJcNocSGMxA7PUCiygSwQTHc1SWcMeOk@mail.gmail.com","threadId":"25124","inReplyTo":"AANLkTik1X0i-OYZCxokw-W3Kt+vEDtBvFeCwQU3q40ap@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-17T00:32:13Z","receivedAt":"2010-09-17T00:32:13Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"On Thu, Sep 16, 2010 at 5:26 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> 2010/9/17 Ævar Arnfjörð Bjarmason <avarab@gmail.com>:\n>> On Thu, Sep 16, 2010 at 20:53, Pat Notz <patnotz@gmail.com> wrote:\n>>> GCC 4.4.4 on MacOS warns about potential use of uninitialized memory.\n>>>\n>>> Signed-off-by: Pat Notz <patnotz@gmail.com>\n>>> ---\n>>>  dir.c |    2 +-\n>>>  1 files changed, 1 insertions(+), 1 deletions(-)\n>>>\n>>> diff --git a/dir.c b/dir.c\n>>> index 133f472..d1e5e5e 100644\n>>> --- a/dir.c\n>>> +++ b/dir.c\n>>> @@ -232,7 +232,7 @@ int add_excludes_from_file_to_list(const char *fname,\n>>>  {\n>>>        struct stat st;\n>>>        int fd, i;\n>>> -       size_t size;\n>>> +       size_t size = 0;\n>>>        char *buf, *entry;\n>>\n>> What does the GCC warning say exactl? I.e. what line does it complain\n>> about?\n\nHere's the output:\n\nmake V=1 -j2 all\ngcc -o dir.o -c   -g -O2 -Wall -I. -I/opt/local/include\n-DUSE_ST_TIMESPEC  -DSHA1_HEADER='<openssl/sha.h>'  -DNO_MEMMEM  dir.c\ndir.c: In function 'add_excludes_from_file_to_list':\ndir.c:235: warning: 'size' may be used uninitialized in this function\n\n\n>>\n>> Maybe this is a logic error introduced in v1.7.0-rc0~25^2? I haven't\n>> checked.\n>\n> I don't see any case that \"size\" can be used uninitialized. Maybe the\n> compiler was confused by\n>\n> if (!check_index ||\n>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)\n>        return -1;\n>\n\nNo, line 245: if(size==0)\n\n> I wouldn't hurt though to initialize it early, even just to stop the\n> compiler from complaining.\n> --\n> Duy\n>\n"},{"id":"150859","messageId":"AANLkTikbd-RQtRQWta+_Ogdicsz-1gFLnXaDYzh3wAfG@mail.gmail.com","threadId":"25124","inReplyTo":"AANLkTin52McRcJcNocSGMxA7PUCiygSwQTHc1SWcMeOk@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2010-09-17T01:04:09Z","receivedAt":"2010-09-17T01:04:09Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:\n>> I don't see any case that \"size\" can be used uninitialized. Maybe the\n>> compiler was confused by\n>>\n>> if (!check_index ||\n>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)\n>>        return -1;\n>>\n>\n> No, line 245: if(size==0)\n\nThe only chance for that line to be executed is read_skip_*() is\nexecuted and returns non-NULL buf. read_skip*() returns a non-NULL\nbuffer at the end of function and does set size right before\nreturning.\n\nTo me it looks like a false alarm. But again, no objection to the patch.\n-- \nDuy\n"},{"id":"150860","messageId":"AANLkTinfgZMuap+hiji3zH6fL4aOS-FrfgxPJfVE1xO6@mail.gmail.com","threadId":"25124","inReplyTo":"AANLkTikbd-RQtRQWta+_Ogdicsz-1gFLnXaDYzh3wAfG@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-17T01:13:06Z","receivedAt":"2010-09-17T01:13:06Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"On Thu, Sep 16, 2010 at 7:04 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n> On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:\n>>> I don't see any case that \"size\" can be used uninitialized. Maybe the\n>>> compiler was confused by\n>>>\n>>> if (!check_index ||\n>>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)\n>>>        return -1;\n>>>\n>>\n>> No, line 245: if(size==0)\n>\n> The only chance for that line to be executed is read_skip_*() is\n> executed and returns non-NULL buf. read_skip*() returns a non-NULL\n> buffer at the end of function and does set size right before\n> returning.\n>\n> To me it looks like a false alarm. But again, no objection to the patch.\n\nI agree that it's a false alarm which is why I wasn't too interested\nin looking into it very deeply.  Just looking to keep the code warning\nfree is all.\n\n> --\n> Duy\n>\n"},{"id":"150897","messageId":"AANLkTinMQ49hPPatgCmxZW6PbU_N8963-XuV3k5f29E2@mail.gmail.com","threadId":"25124","inReplyTo":"AANLkTinfgZMuap+hiji3zH6fL4aOS-FrfgxPJfVE1xO6@mail.gmail.com","subject":"Re: [PATCH 1/2] dir.c: fix uninitialized memory warning","fromName":"Pat Notz","fromEmail":"patnotz@gmail.com","sentAt":"2010-09-17T17:23:13Z","receivedAt":"2010-09-17T17:23:13Z","isPatch":true,"sender":{"key":"patnotz@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45364?v=4"},"body":"For anyone who care, this warning was actually emitted by the version\nof GCC that ships with MacOS 10.5: i686-apple-darwin9-gcc-4.0.1 (GCC)\n4.0.1 (Apple Inc. build 5493).\n\nGCC 4.4.4 does *not* git this warning.\n\nSorry for the confusion, my IDE was using a different $PATH than my shell.\n\n\nOn Thu, Sep 16, 2010 at 7:13 PM, Pat Notz <patnotz@gmail.com> wrote:\n> On Thu, Sep 16, 2010 at 7:04 PM, Nguyen Thai Ngoc Duy <pclouds@gmail.com> wrote:\n>> On Fri, Sep 17, 2010 at 10:32 AM, Pat Notz <patnotz@gmail.com> wrote:\n>>>> I don't see any case that \"size\" can be used uninitialized. Maybe the\n>>>> compiler was confused by\n>>>>\n>>>> if (!check_index ||\n>>>>    (buf = read_skip_worktree_file_from_index(fname, &size)) == NULL)\n>>>>        return -1;\n>>>>\n>>>\n>>> No, line 245: if(size==0)\n>>\n>> The only chance for that line to be executed is read_skip_*() is\n>> executed and returns non-NULL buf. read_skip*() returns a non-NULL\n>> buffer at the end of function and does set size right before\n>> returning.\n>>\n>> To me it looks like a false alarm. But again, no objection to the patch.\n>\n> I agree that it's a false alarm which is why I wasn't too interested\n> in looking into it very deeply.  Just looking to keep the code warning\n> free is all.\n>\n>> --\n>> Duy\n>>\n>\n"}]}