threads / patch / 46641

patchrepository: fix a sparse 'using integer a NULL pointer' warning

Subject: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

## tl;dr

4 messages between Aug 21, 2017 and Aug 22, 2017. Diffs are folded; open one to read it.

replies: 3people: 4as markdown or json

Ramsay Jones· Aug 21, 2017, 15:48 UTC · lore

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 <ramsay@ramsayjones.plus.com>
---

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(-)

Show changes to repository.c +1 −1

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· Aug 22, 2017, 02:11 UTC · re: Ramsay Jones · lore

Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

On Mon, Aug 21, 2017 at 04:48:42PM +0100, Ramsay Jones wrote:

Show 16 quoted lines

> 
> 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 <ramsay@ramsayjones.plus.com>
> ---
> 
> 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· Aug 22, 2017, 17:35 UTC · re: Ramsay Jones · lore

Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

Am 21.08.2017 um 17:48 schrieb Ramsay Jones:

Show 35 quoted lines

> 
> 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 <ramsay@ramsayjones.plus.com>
> ---
> 
> 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· Aug 22, 2017, 18:12 UTC · re: René Scharfe · lore

Re: [PATCH] repository: fix a sparse 'using integer a NULL pointer' warning

René Scharfe <l.s.r@web.de> writes:

Show 19 quoted lines

>> 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.