# [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning 4 messages from 2017-08-21 to 2017-08-22. Participants: Ramsay Jones, brian m. carlson, René Scharfe, Junio C Hamano. Thread: https://gitlist.dev/t/46641 ## Ramsay Jones, 2017-08-21 15:48 Subject: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning Message-ID: <91150cfc-3271-16b0-33d3-9a4e149dc9fe@ramsayjones.plus.com> URL: https://gitlist.dev/e/91150cfc-3271-16b0-33d3-9a4e149dc9fe%40ramsayjones.plus.com ``` Commit 67a9dfcc00 ("hash-algo: integrate hash algorithm support with repo setup", 21-08-2017) added a 'const struct git_hash_algo *hash_algo' field to the repository structure, without modifying the initializer of the 'the_repo' variable. This does not actually introduce a bug, since the '0' initializer for the 'ignore_env:1' bit-field is interpreted as a NULL pointer (hence the warning), and the final field (now with no initializer) receives a default '0'. Signed-off-by: Ramsay Jones --- Hi Brian, If you need to re-roll your 'bc/hash-algo' branch, could you please squash this into the relevant patch. Thanks! ATB, Ramsay Jones repository.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/repository.c b/repository.c index 01af20dee..ceef73614 100644 --- a/repository.c +++ b/repository.c @@ -5,7 +5,7 @@ /* The main repository */ static struct repository the_repo = { - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, 0, 0 + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0 }; struct repository *the_repository = &the_repo; -- 2.14.0 ``` ## brian m. carlson, 2017-08-22 02:11 Subject: Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning Message-ID: <20170822021124.4bmlzpdocgheo55b@genre.crustytoothpaste.net> URL: https://gitlist.dev/e/20170822021124.4bmlzpdocgheo55b%40genre.crustytoothpaste.net In-Reply-To: <91150cfc-3271-16b0-33d3-9a4e149dc9fe@ramsayjones.plus.com> ``` On Mon, Aug 21, 2017 at 04:48:42PM +0100, Ramsay Jones wrote: > > Commit 67a9dfcc00 ("hash-algo: integrate hash algorithm support with > repo setup", 21-08-2017) added a 'const struct git_hash_algo *hash_algo' > field to the repository structure, without modifying the initializer > of the 'the_repo' variable. This does not actually introduce a bug, > since the '0' initializer for the 'ignore_env:1' bit-field is > interpreted as a NULL pointer (hence the warning), and the final field > (now with no initializer) receives a default '0'. > > Signed-off-by: Ramsay Jones > --- > > Hi Brian, > > If you need to re-roll your 'bc/hash-algo' branch, could you please > squash this into the relevant patch. Of course, thanks. I'll almost certainly be doing a reroll since it's an RFC series and I expect to get more feedback on it. -- brian m. carlson / brian with sandals: Houston, Texas, US https://www.crustytoothpaste.net/~bmc | My opinion only OpenPGP: https://keybase.io/bk2204 ``` ## René Scharfe, 2017-08-22 17:35 Subject: Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning Message-ID: <9e45674c-bdc2-003e-daa4-79a3903dde8f@web.de> URL: https://gitlist.dev/e/9e45674c-bdc2-003e-daa4-79a3903dde8f%40web.de In-Reply-To: <91150cfc-3271-16b0-33d3-9a4e149dc9fe@ramsayjones.plus.com> ``` Am 21.08.2017 um 17:48 schrieb Ramsay Jones: > > Commit 67a9dfcc00 ("hash-algo: integrate hash algorithm support with > repo setup", 21-08-2017) added a 'const struct git_hash_algo *hash_algo' > field to the repository structure, without modifying the initializer > of the 'the_repo' variable. This does not actually introduce a bug, > since the '0' initializer for the 'ignore_env:1' bit-field is > interpreted as a NULL pointer (hence the warning), and the final field > (now with no initializer) receives a default '0'. > > Signed-off-by: Ramsay Jones > --- > > Hi Brian, > > If you need to re-roll your 'bc/hash-algo' branch, could you please > squash this into the relevant patch. > > Thanks! > > ATB, > Ramsay Jones > > repository.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/repository.c b/repository.c > index 01af20dee..ceef73614 100644 > --- a/repository.c > +++ b/repository.c > @@ -5,7 +5,7 @@ > > /* The main repository */ > static struct repository the_repo = { > - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, 0, 0 > + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0 This line yells out "designated initializer" to me: + .index = &the_index > }; > struct repository *the_repository = &the_repo; > > ``` ## Junio C Hamano, 2017-08-22 18:12 Subject: Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning Message-ID: URL: https://gitlist.dev/e/xmqqwp5v320m.fsf%40gitster.mtv.corp.google.com In-Reply-To: <9e45674c-bdc2-003e-daa4-79a3903dde8f@web.de> ``` René Scharfe writes: >> diff --git a/repository.c b/repository.c >> index 01af20dee..ceef73614 100644 >> --- a/repository.c >> +++ b/repository.c >> @@ -5,7 +5,7 @@ >> >> /* The main repository */ >> static struct repository the_repo = { >> - NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, 0, 0 >> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, &the_index, NULL, 0, 0 > > This line yells out "designated initializer" to me: > > + .index = &the_index > >> }; >> struct repository *the_repository = &the_repo; >> >> Yes, but let's hold it off for a while, until at least what is already in the tip of 'master' graduates to a released version at the end of the current cycle. We picked reasonably quiecent parts of the codebase and implanted uses of a few C99 features to ensure that we get complaints and requests for revert from people on exotic platforms, so that we can back them out easily. cbc0f81d ("strbuf: use designated initializers in STRBUF_INIT", 2017-07-10) does the designated initializer for struct members. 512f41cf ("clean.c: use designated initializer", 2017-07-14) does the same for array elements. In addition, e1327023 ("grep: refactor the concept of "grep source" into an object", 2012-02-02) inadvertently introduced the use of trailing comma in enum definition about 5 years ago, so we know that one is safe to use. ```