threads / patch / 28852

patchCompatibility: declare strtoimax() under NO_STRTOUMAX

Subject: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

## tl;dr

6 messages between Nov 5, 2011 and Nov 6, 2011. Diffs are folded; open one to read it.

replies: 5people: 4as markdown or json

Johannes Sixt· Nov 5, 2011, 15:37 UTC · lore

Commit f696543d (Add strtoimax() compatibility function) introduced an implementation of the function, but forgot to add a declaration.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 git-compat-util.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

Show changes to git-compat-util.h +2 −0

diff --git a/git-compat-util.h b/git-compat-util.h
index feb6f8e..4efef46 100644
--- a/git-compat-util.h
+++ b/git-compat-util.h
@@ -354,6 +354,8 @@ extern size_t gitstrlcpy(char *, const char *, size_t);
 #ifdef NO_STRTOUMAX
 #define strtoumax gitstrtoumax
 extern uintmax_t gitstrtoumax(const char *, char **, int);
+#define strtoimax gitstrtoimax
+extern intmax_t gitstrtoimax(const char *, char **, int);
 #endif
 
 #ifdef NO_STRTOK_R

-- 
1.7.7.1.1608.gd424d

Nix· Nov 5, 2011, 15:38 UTC · re: Johannes Sixt · lore

Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

On 5 Nov 2011, Johannes Sixt said:

> Commit f696543d (Add strtoimax() compatibility function) introduced an
> implementation of the function, but forgot to add a declaration.

Oh, my apologies. (How did my testing miss that? No -Wall, I bet.)

-- 
NULL && (void)

Johannes Sixt· Nov 5, 2011, 16:34 UTC · re: Johannes Sixt · lore

Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

Am 05.11.2011 16:37, schrieb Johannes Sixt:

> Commit f696543d (Add strtoimax() compatibility function) introduced an
> implementation of the function, but forgot to add a declaration.

On second thought, I'm puzzled: Without this patch and without noticing the warning that strtoimax() was not declared, I had built with NO_STRTOUMAX on MinGW before, and the build succeeded. This means that even though MinGW's headers are not C99, we must have pulled in function strtoimax() from somewhere. I'll investigate later this weekend.

Anyway, this patch does not just add a declaration for the function, but also redirects strtoimax to gitstrtoimax, which is a bit more than the commit message claims. Without this patch, topic na/strtoimax should not build on a non-C99 environment. Can you verify this claim?

-- Hannes

Show 19 quoted lines

> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  git-compat-util.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/git-compat-util.h b/git-compat-util.h
> index feb6f8e..4efef46 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -354,6 +354,8 @@ extern size_t gitstrlcpy(char *, const char *, size_t);
>  #ifdef NO_STRTOUMAX
>  #define strtoumax gitstrtoumax
>  extern uintmax_t gitstrtoumax(const char *, char **, int);
> +#define strtoimax gitstrtoimax
> +extern intmax_t gitstrtoimax(const char *, char **, int);
>  #endif
>  
>  #ifdef NO_STRTOK_R

Ramsay Jones· Nov 6, 2011, 18:21 UTC · re: Johannes Sixt · lore

Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

Johannes Sixt wrote:

Show 9 quoted lines

> Am 05.11.2011 16:37, schrieb Johannes Sixt:
>> Commit f696543d (Add strtoimax() compatibility function) introduced an
>> implementation of the function, but forgot to add a declaration.
> 
> On second thought, I'm puzzled: Without this patch and without noticing
> the warning that strtoimax() was not declared, I had built with
> NO_STRTOUMAX on MinGW before, and the build succeeded. This means that
> even though MinGW's headers are not C99, we must have pulled in function
> strtoimax() from somewhere. I'll investigate later this weekend.

Yes, I've noticed the same thing, viz:

    ramsay (pu)$ find /mingw/include -type f | xargs egrep 'strto(imax|umax|ull)'
    /mingw/include/inttypes.h:intmax_t __cdecl __MINGW_NOTHROW strtoimax (const char
    * __restrict__ nptr,
    /mingw/include/inttypes.h:uintmax_t __cdecl __MINGW_NOTHROW strtoumax (const cha
    r* __restrict__ nptr,
    /mingw/include/stdlib.h:unsigned long long  __cdecl __MINGW_NOTHROW strtoull (co
    nst char* __restrict__, char** __restrict__, int);

    ramsay (pu)$ nm /mingw/lib/libmingwex.a | egrep 'strto(imax|umax|ull)'
    strtoimax.o:
    00000000 T _strtoimax
    strtoumax.o:
    00000000 T _strtoull
    00000000 T _strtoumax

    ramsay (pu)$

So, I assume, an earlier version of MinGW did not have the inttypes.h header file and the NO_STRTOUMAX etc. settings are stale; dunno ;-)

In any event, I guess an '#include <inttypes.h>' on MinGW will be part of the solution. [Without breaking the MSVC build, of course; see the NO_INTTYPES_H make variable.] However, I haven't given it too much thought, so take that with a bucket of salt!

Note, also, that very similar comments apply to NO_LIBGEN_H; viz:

    ramsay (pu)$ find /mingw/include -type f | xargs egrep 'char \*(dir|base)name'
    /mingw/include/libgen.h:extern __cdecl __MINGW_NOTHROW char *basename (char *);
    /mingw/include/libgen.h:extern __cdecl __MINGW_NOTHROW char *dirname  (char *);

    ramsay (pu)$ nm /mingw/lib/libmingwex.a | egrep '(dir|base)name'
    basename.o:
    00000000 T _basename
    dirname.o:
    00000000 T _dirname

    ramsay (pu)$

[having said that, I have a patch that adds a compat/dirname.c and modifies the existing compat/basename.c; the MinGW dirname()/basename() functions have problems.]

HTH

ATB, Ramsay Jones

Junio C Hamano· Nov 6, 2011, 06:10 UTC · re: Johannes Sixt · lore

Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

Johannes Sixt <j6t@kdbg.org> writes:

Show 5 quoted lines

> Commit f696543d (Add strtoimax() compatibility function) introduced an
> implementation of the function, but forgot to add a declaration.
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---

Thanks, but I think f696543d is v1.7.6 and not that patch. I hope you do not mind if I just squashed this in, instead of leaving it as a separate patch.

Show 16 quoted lines

>  git-compat-util.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/git-compat-util.h b/git-compat-util.h
> index feb6f8e..4efef46 100644
> --- a/git-compat-util.h
> +++ b/git-compat-util.h
> @@ -354,6 +354,8 @@ extern size_t gitstrlcpy(char *, const char *, size_t);
>  #ifdef NO_STRTOUMAX
>  #define strtoumax gitstrtoumax
>  extern uintmax_t gitstrtoumax(const char *, char **, int);
> +#define strtoimax gitstrtoimax
> +extern intmax_t gitstrtoimax(const char *, char **, int);
>  #endif
>  
>  #ifdef NO_STRTOK_R

Johannes Sixt· Nov 6, 2011, 08:53 UTC · re: Junio C Hamano · lore

Re: [PATCH na/strtoimax] Compatibility: declare strtoimax() under NO_STRTOUMAX

Am 06.11.2011 07:10, schrieb Junio C Hamano:

Show 9 quoted lines

> Johannes Sixt <j6t@kdbg.org> writes:
> 
>> Commit f696543d (Add strtoimax() compatibility function) introduced an
>> implementation of the function, but forgot to add a declaration.
>>
>> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
>> ---
> 
> Thanks, but I think f696543d is v1.7.6 and not that patch.

Oops, sorry for that.

>  I hope you do
> not mind if I just squashed this in, instead of leaving it as a separate
> patch.

I do not mind at all.

-- Hannes