git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: /* compiler workaround */ - what was the issue?

From
Philip Oakley <philipoakley@iee.org>
Date
May 6, 2016, 18:33 UTC
Message-ID
<0727E275927743DF94CFEC8615D7E90B@PhilipOakley>
In-Reply-To
<572CDCFF.9050607@ramsayjones.plus.com>
From: "Ramsay Jones" <ramsay@ramsayjones.plus.com>
Show 44 quoted lines
> On 06/05/16 14:15, Philip Oakley wrote:
>> From: "Duy Nguyen" <pclouds@gmail.com>
>>> On Fri, May 6, 2016 at 4:41 AM, Junio C Hamano <gitster@pobox.com> 
>>> wrote:
>>>> "Philip Oakley" <philipoakley@iee.org> writes:
>>>>
>>>>>     int saved_namelen = saved_namelen; /* compiler workaround */
>>>>>
>>>>> Which then becomes an MSVC compile warning C4700: uninitialized local
>>>>> variable.
>>>>>
>>>>> I'm wondering what was the compiler workaround being referred to? i.e. 
>>>>> why
>>>>> does it need that tweak? There's no mention of the reason in the 
>>>>> commit
>>>>> message.
>>>>
>>>> That was a fairly well-known workaround for GCC that issues a false
>>>> warning that variable is used before initialized.  I thought we
>>>> stopped using it several years ago in new code after doing a bulk
>>>> sanitizing
>>>
>>> I guess that's 803a777 (cat-file: Fix an gcc -Wuninitialized warning -
>>> 2013-03-26) and more commits around that time. The split-index commit
>>> is in 2014. I must have missed the trend.
>>>
>>>> (I think the new recommended workaround was to initialise
>>>> such a variable to the nil value like '0' for integers).
>>>
>>> Yep. First Jeff removed the " = xxx" part from "xxx = xxx" then Ramsay
>>> added the " = NULL" back. So we probably just do "int saved_namelen =
>>> 0;" in this case.
>>> -- 
>> Thanks,
>>
>> I'll try and work up a patch - probably next week as I'm away for the 
>> weekend.
>
> Yeah, I don't remember why these were left over from the previous
> attempt to clean these up (maybe they conflicted with in-flight
> topics?), but I have had a patch hanging around ... :-D
>
> The patch below applies to master (I haven't checked for any more
> additions).
Looks good to me. Catches those I've seen.
Have a good weekend all.
Show 87 quoted lines
>
> ATB,
> Ramsay Jones
>
> -- >8 --
> From: Ramsay Jones <ramsay@ramsayjones.plus.com>
> Subject: [PATCH] -Wuninitialized: remove a gcc specific workaround
>
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
> builtin/rev-list.c | 2 +-
> fast-import.c      | 4 ++--
> merge-recursive.c  | 2 +-
> read-cache.c       | 2 +-
> 4 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/builtin/rev-list.c b/builtin/rev-list.c
> index 275da0d..deae1f3 100644
> --- a/builtin/rev-list.c
> +++ b/builtin/rev-list.c
> @@ -377,7 +377,7 @@ int cmd_rev_list(int argc, const char **argv, const 
> char *prefix)
>  mark_edges_uninteresting(&revs, show_edge);
>
>  if (bisect_list) {
> - int reaches = reaches, all = all;
> + int reaches = 0, all = 0;
>
>  revs.commits = find_bisection(revs.commits, &reaches, &all,
>        bisect_find_all);
> diff --git a/fast-import.c b/fast-import.c
> index 9fc7093..ca66d80 100644
> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -2935,7 +2935,7 @@ static void cat_blob(struct object_entry *oe, 
> unsigned char sha1[20])
>
> static void parse_get_mark(const char *p)
> {
> - struct object_entry *oe = oe;
> + struct object_entry *oe = NULL;
>  char output[42];
>
>  /* get-mark SP <object> LF */
> @@ -2952,7 +2952,7 @@ static void parse_get_mark(const char *p)
>
> static void parse_cat_blob(const char *p)
> {
> - struct object_entry *oe = oe;
> + struct object_entry *oe = NULL;
>  unsigned char sha1[20];
>
>  /* cat-blob SP <object> LF */
> diff --git a/merge-recursive.c b/merge-recursive.c
> index 06d31ed..9cecc24 100644
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -1897,7 +1897,7 @@ int merge_recursive(struct merge_options *o,
> {
>  struct commit_list *iter;
>  struct commit *merged_common_ancestors;
> - struct tree *mrtree = mrtree;
> + struct tree *mrtree = NULL;
>  int clean;
>
>  if (show(o, 4)) {
> diff --git a/read-cache.c b/read-cache.c
> index d9fb78b..978d6b6 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1870,7 +1870,7 @@ static int ce_write_entry(git_SHA_CTX *c, int fd, 
> struct cache_entry *ce,
> {
>  int size;
>  struct ondisk_cache_entry *ondisk;
> - int saved_namelen = saved_namelen; /* compiler workaround */
> + int saved_namelen = 0;
>  char *name;
>  int result;
>
> -- 
> 2.8.0
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Previous: Ramsay JonesNext: Junio C Hamano
Message 6 of 15 in “/* compiler workaround */ - what was the issue?”
  1. Philip OakleyMay 5, 2016
  2. Junio C HamanoMay 5, 2016
  3. Duy NguyenMay 6, 2016
  4. Philip OakleyMay 6, 2016
  5. Ramsay JonesMay 6, 2016
  6. Philip OakleyMay 6, 2016
  7. Junio C HamanoMay 6, 2016
  8. Marc BranchaudMay 6, 2016
  9. Junio C HamanoMay 6, 2016
  10. Stefan BellerMay 6, 2016
  11. Philip OakleyMay 9, 2016
  12. Randall S. BeckerMay 9, 2016
  13. Marc BranchaudMay 6, 2016
  14. Ramsay JonesMay 6, 2016
  15. Ramsay JonesMay 6, 2016

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.