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

[Ramsay Jones]

On 29/11/17 01:35, brian m. carlson wrote:
> On Tue, Nov 28, 2017 at 03:01:19AM +, Ramsay Jones wrote:
>>
>> Commit 78a6766802 ("Integrate hash algorithm support with repo setup",
>> 2017-11-12) 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 Junio,
>>
>> I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but
>> now that it has been merged into the 'next' branch, sparse is barking on
>> that branch too. This patch (slightly different to the last one) applies
>> on top of 'next'.
> 
> Thanks for the patch; it's obviously correct.  I'll get sparse set up
> for future patch series.

Heh, at this point I usually remark that you would need to build
sparse from source, since all the packaged versions are so old
as to be virtually useless (to run over git).

That may still be the case, depending on your distro, since there
was a new release a couple of months ago. However, you know how
long it takes to get into some distros! ;-)

[I _think_ it may be in Debian Unstable]

Oh, the release (v0.5.1) was tagged on 17 Aug 2017.
(I'm currently running v0.5.1-30-gf1e4ba1, built from source).

ATB,
Ramsay Jones

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

[brian m. carlson]

On Tue, Nov 28, 2017 at 03:01:19AM +, Ramsay Jones wrote:
> 
> Commit 78a6766802 ("Integrate hash algorithm support with repo setup",
> 2017-11-12) 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 Junio,
> 
> I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but
> now that it has been merged into the 'next' branch, sparse is barking on
> that branch too. This patch (slightly different to the last one) applies
> on top of 'next'.

Thanks for the patch; it's obviously correct.  I'll get sparse set up
for future patch series.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204


signature.asc
Description: PGP signature

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

[Junio C Hamano]

Ramsay Jones  writes:

> Hi Junio,
>
> I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but
> now that it has been merged into the 'next' branch, sparse is barking on
> that branch too. This patch (slightly different to the last one) applies
> on top of 'next'.

Thanks.  Will queue on that topic.

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

[Ramsay Jones]

Commit 78a6766802 ("Integrate hash algorithm support with repo setup",
2017-11-12) 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 Junio,

I don't recall Brian doing a re-roll of the 'bc/hash-algo' branch[1], but
now that it has been merged into the 'next' branch, sparse is barking on
that branch too. This patch (slightly different to the last one) applies
on top of 'next'.

ATB,
Ramsay Jones

[1] 
https://public-inbox.org/git/%3c91150cfc-3271-16b0-33d3-9a4e149dc...@ramsayjones.plus.com%3E/

 repository.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/repository.c b/repository.c
index c6ceb9f9e..998413b8b 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, _index, 0, 0
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
0, 0
 };
 struct repository *the_repository = _repo;
 
-- 
2.15.0

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

[Junio C Hamano]

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, _index, 0, 0
>> +NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
>> 0, 0
>
> This line yells out "designated initializer" to me:
>
> + .index = _index
>
>>   };
>>   struct repository *the_repository = _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.

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

[René Scharfe]

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, _index, 0, 0
> + NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
> 0, 0

This line yells out "designated initializer" to me:

+   .index = _index

>   };
>   struct repository *the_repository = _repo;
>   
> 

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

[brian m. carlson]

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


signature.asc
Description: PGP signature

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

[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, _index, 0, 0
+   NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL, _index, NULL, 
0, 0
 };
 struct repository *the_repository = _repo;
 
-- 
2.14.0