50a6c8ef - xmalloc_array

classic Classic list List threaded Threaded
18 messages Options
Reply | Threaded
Open this post in threaded view
|

50a6c8ef - xmalloc_array

Torsten Bögershausen-2
I haven't digged further,
but trying to verify t9115 under Windows gave this:


compat/mingw.c: In function 'mingw_spawnve_fd':
compat/mingw.c:1072:10: warning: implicit declaration of function
'xmalloc_array' [-Wimplicit-function-declaration]
   wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
           ^
compat/mingw.c:1072:8: warning: assignment makes pointer from integer without a
cast [-Wint-conversion]
   wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
         ^
--- and later ---

libgit.a(mingw.o): In function `mingw_spawnve_fd':
..... compat/mingw.c:1072: undefined reference to `xmalloc_array'
collect2.exe: error: ld returned 1 exit status
Makefile:2014: recipe for target 'git-credential-store.exe' failed
make: *** [git-credential-store.exe] Error 1


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

[PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Jeff King
On Mon, Feb 29, 2016 at 07:30:02AM +0100, Torsten Bögershausen wrote:

> I haven't digged further,
> but trying to verify t9115 under Windows gave this:
>
> compat/mingw.c: In function 'mingw_spawnve_fd':
> compat/mingw.c:1072:10: warning: implicit declaration of function
> 'xmalloc_array' [-Wimplicit-function-declaration]
>   wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
>           ^

Yikes.

-- >8 --
Subject: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Commit 50a6c8e (use st_add and st_mult for allocation size
computation, 2016-02-22) fixed up many xmalloc call-sites
including ones in compat/mingw.c.

But I screwed up one of them, which was half-converted to
ALLOC_ARRAY, using a very early prototype of the function.
And I never caught it because I don't build on Windows.

Signed-off-by: Jeff King <[hidden email]>
---
I think this means "master" is broken for mingw builds.

Sorry, Windows people, for breaking your build. I'm happy to hold back
such repo-wide cleanups from the mingw code in the future, since I can't
actually compile them. But the flipside is that if I _do_ improve
things, you don't get the benefit until somebody manually ports it over.

 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index cfedcf9..54c82ec 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
  free(quoted);
  }
 
- wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
+ ALLOC_ARRAY(wargs, st_add(st_mult(2, args.len), 1));
  xutftowcs(wargs, args.buf, 2 * args.len + 1);
  strbuf_release(&args);
 
--
2.8.0.rc0.276.gddf4100

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Torsten Bögershausen-2
Thanks for the fast-patch.

I manually copied the patch, But there are more probles, it seems.

$ git diff
diff --git a/compat/mingw.c b/compat/mingw.c
index cfedcf9..b1163f2 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char
**argv, char **deltaen
                         free(quoted);
         }

-       wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
+       wargs = ALLOC_ARRAY(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
         xutftowcs(wargs, args.buf, 2 * args.len + 1);
         strbuf_release(&args);


tb@torbogwm MINGW64 ~/Users/tb/projects/git/tb ((2273582...))
$ make
     CC compat/mingw.o
In file included from compat/mingw.c:1:0:
compat/mingw.c: In function 'mingw_spawnve_fd':
compat/../git-compat-util.h:769:60: error: invalid type argument of unary '*'
(have 'size_t {aka long long unsigned int}')
  #define ALLOC_ARRAY(x, alloc) (x) = xmalloc(st_mult(sizeof(*(x)), (alloc)))
                                                             ^
compat/mingw.c:1072:10: note: in expansion of macro 'ALLOC_ARRAY'
   wargs = ALLOC_ARRAY(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
           ^
Makefile:1948: recipe for target 'compat/mingw.o' failed
make: *** [compat/mingw.o] Error 1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Jeff King
On Mon, Feb 29, 2016 at 10:56:22AM +0100, Torsten Bögershausen wrote:

> Thanks for the fast-patch.
>
> I manually copied the patch, But there are more probles, it seems.
>
> $ git diff
> diff --git a/compat/mingw.c b/compat/mingw.c
> index cfedcf9..b1163f2 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const
> char **argv, char **deltaen
>                         free(quoted);
>         }
>
> -       wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
> +       wargs = ALLOC_ARRAY(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
>         xutftowcs(wargs, args.buf, 2 * args.len + 1);
>         strbuf_release(&args);

Argh. Let me write "git commit -a" on the inside of my brown paper bag,
so that I actually send out the fix sitting in my working tree, not the
half-finished thing I ran "git add" on.

-- >8 --
Subject: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Commit 50a6c8e (use st_add and st_mult for allocation size
computation, 2016-02-22) fixed up many xmalloc call-sites
including ones in compat/mingw.c.

But I screwed up one of them, which was half-converted to
ALLOC_ARRAY, using a very early prototype of the function.
And I never caught it because I don't build on Windows.

Signed-off-by: Jeff King <[hidden email]>
---
 compat/mingw.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/compat/mingw.c b/compat/mingw.c
index cfedcf9..54c82ec 100644
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
  free(quoted);
  }
 
- wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
+ ALLOC_ARRAY(wargs, st_add(st_mult(2, args.len), 1));
  xutftowcs(wargs, args.buf, 2 * args.len + 1);
  strbuf_release(&args);
 
--
2.8.0.rc0.276.gddf4100


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Compiler warning under cygwin/mingw (was: fix for 50a6c8e)

Torsten Bögershausen-2
That compiles OK, thanks.


Sorry for high-jacking this thread, but while compiling under CYGWIN,
found one warning:

    LINK git-credential-store.exe
     CC daemon.o
daemon.c: In function ‘drop_privileges’:
daemon.c:1136:15: warning: implicit declaration of function ‘initgroups’
[-Wimplicit-function-declaration]
   if (cred && (initgroups(cred->pass->pw_name, cred->gid) ||
                ^

t9115 doesn't pass,  NTFS doesn't like the non-UTF filenames, it seams.
Probably the same problem as under Mac OS /HFS+-
-----------------------

And MINGW is not happy for other reasons:

builtin/rev-parse.c: In function 'cmd_rev_parse':
builtin/rev-parse.c:775:12: warning: implicit declaration of function
'realpath' [-Wimplicit-function-declaration]
        if (!realpath(gitdir, absolute_path))
             ^
     CC builtin/revert.o


     CC builtin/write-tree.o
     LINK git.exe
builtin/rev-parse.o: In function `cmd_rev_parse':
C:\Users\tb\projects\git\tb/builtin/rev-parse.c:775: undefined reference to
`realpath'
collect2.exe: error: ld returned 1 exit status
Makefile:1725: recipe for target 'git.exe' failed
make: *** [git.exe] Error 1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw (was: fix for 50a6c8e)

Jeff King
On Mon, Feb 29, 2016 at 11:40:59AM +0100, Torsten Bögershausen wrote:

> That compiles OK, thanks.
>
> Sorry for high-jacking this thread, but while compiling under CYGWIN,
> found one warning:
>
>    LINK git-credential-store.exe
>     CC daemon.o
> daemon.c: In function ‘drop_privileges’:
> daemon.c:1136:15: warning: implicit declaration of function ‘initgroups’
> [-Wimplicit-function-declaration]
>   if (cred && (initgroups(cred->pass->pw_name, cred->gid) ||

Interesting that it doesn't later complain in the link step. :)

You should probably be compiling with the NO_INITGROUPS knob on that
platform.

> t9115 doesn't pass,  NTFS doesn't like the non-UTF filenames, it seams.
> Probably the same problem as under Mac OS /HFS+-
> -----------------------

No comment from me on that one.

> And MINGW is not happy for other reasons:
>
> builtin/rev-parse.c: In function 'cmd_rev_parse':
> builtin/rev-parse.c:775:12: warning: implicit declaration of function
> 'realpath' [-Wimplicit-function-declaration]
>        if (!realpath(gitdir, absolute_path))
>             ^

I guess you're building "pu"; that is only in sg/completion-updates. I
don't know if our custom real_path() would suffice there. You might want
to ping the author. The patch is:

  http://article.gmane.org/gmane.comp.version-control.git/287462

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw

Ramsay Jones-2
In reply to this post by Torsten Bögershausen-2


On 29/02/16 10:40, Torsten Bögershausen wrote:

> That compiles OK, thanks.
>
>
> Sorry for high-jacking this thread, but while compiling under CYGWIN,
> found one warning:
>
>    LINK git-credential-store.exe
>     CC daemon.o
> daemon.c: In function ‘drop_privileges’:
> daemon.c:1136:15: warning: implicit declaration of function ‘initgroups’ [-Wimplicit-function-declaration]
>   if (cred && (initgroups(cred->pass->pw_name, cred->gid) ||

Yeah, this has been there for a while - except it depends on which version
of the header files you have. (Some may not see the warning).

I have 'fixed' this twice before, then updated my installation and
a change to the system headers broke it again! (The headers are
currently 'broken'). So, I got tired of fixing it up and have left
it a while - you never know a new update may fix it! ;-)

[I personally don't use the git daemon on cygwin, so I don't know
if this a problem in practice.]

ATB,
Ramsay Jones

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Johannes Schindelin
In reply to this post by Jeff King
Hi Peff,

On Mon, 29 Feb 2016, Jeff King wrote:

> I think this means "master" is broken for mingw builds.
>
> Sorry, Windows people, for breaking your build. I'm happy to hold back
> such repo-wide cleanups from the mingw code in the future, since I can't
> actually compile them. But the flipside is that if I _do_ improve
> things, you don't get the benefit until somebody manually ports it over.

No, I do not think that you need to hold back cleanups. We will catch such
issues before long, anyway.

Thanks for all your hard work!
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw (was: fix for 50a6c8e)

SZEDER Gábor
In reply to this post by Jeff King
Hi,

> > And MINGW is not happy for other reasons:
> >
> > builtin/rev-parse.c: In function 'cmd_rev_parse':
> > builtin/rev-parse.c:775:12: warning: implicit declaration of function
> > 'realpath' [-Wimplicit-function-declaration]
> >        if (!realpath(gitdir, absolute_path))
> >             ^
>
> I guess you're building "pu"; that is only in sg/completion-updates. I
> don't know if our custom real_path() would suffice there. You might want
> to ping the author. The patch is:
>
>   http://article.gmane.org/gmane.comp.version-control.git/287462

Oh, I was not aware that there is a custom real_path() that is
preferred over the system realpath().  I don't see why our real_path()
would not suffice, it even makes the code a tad shorter.

I will include the patch below in the reroll.

Best,
Gábor


 ----  >8  ----
Subject: [PATCH] fixup! rev-parse: add '--absolute-git-dir' option

---
 builtin/rev-parse.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index 90a4dd6032c0..d6d9a82c77c4 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -762,10 +762,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
  if (!gitdir && !prefix)
  gitdir = ".git";
  if (gitdir) {
- char absolute_path[PATH_MAX];
- if (!realpath(gitdir, absolute_path))
- die_errno(_("unable to get absolute path"));
- puts(absolute_path);
+ puts(real_path(gitdir));
  continue;
  }
  }
--
2.7.2.410.g92cb358

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Junio C Hamano
In reply to this post by Jeff King
Jeff King <[hidden email]> writes:

> On Mon, Feb 29, 2016 at 10:56:22AM +0100, Torsten Bögershausen wrote:
>
>> Thanks for the fast-patch.
>>
>> I manually copied the patch, But there are more probles, it seems.
>>
>> $ git diff
>> diff --git a/compat/mingw.c b/compat/mingw.c
>> index cfedcf9..b1163f2 100644
>> --- a/compat/mingw.c
>> +++ b/compat/mingw.c
>> @@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const
>> char **argv, char **deltaen
>>                         free(quoted);
>>         }
>>
>> -       wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
>> +       wargs = ALLOC_ARRAY(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
>>         xutftowcs(wargs, args.buf, 2 * args.len + 1);
>>         strbuf_release(&args);
>
> Argh. Let me write "git commit -a" on the inside of my brown paper bag,
> so that I actually send out the fix sitting in my working tree, not the
> half-finished thing I ran "git add" on.

Just to make sure that I am not confused, what you wrote below
matches what I received from you two message upthread.

I am assuming that it is intended that the two messages from you
have the same patch, and the assignment of ALLOC_ARRAY to wargs was
a bug Torsten introduced only to his tree when cutting and pasting.

With that assumption, will queue this one (or the original one,
which to me is the same thing).

Thanks.

> -- >8 --
> Subject: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e
>
> Commit 50a6c8e (use st_add and st_mult for allocation size
> computation, 2016-02-22) fixed up many xmalloc call-sites
> including ones in compat/mingw.c.
>
> But I screwed up one of them, which was half-converted to
> ALLOC_ARRAY, using a very early prototype of the function.
> And I never caught it because I don't build on Windows.
>
> Signed-off-by: Jeff King <[hidden email]>
> ---
>  compat/mingw.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/compat/mingw.c b/compat/mingw.c
> index cfedcf9..54c82ec 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -1069,7 +1069,7 @@ static pid_t mingw_spawnve_fd(const char *cmd, const char **argv, char **deltaen
>   free(quoted);
>   }
>  
> - wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
> + ALLOC_ARRAY(wargs, st_add(st_mult(2, args.len), 1));
>   xutftowcs(wargs, args.buf, 2 * args.len + 1);
>   strbuf_release(&args);
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Jeff King
On Mon, Feb 29, 2016 at 11:10:09AM -0800, Junio C Hamano wrote:

> >> -       wargs = xmalloc_array(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
> >> +       wargs = ALLOC_ARRAY(st_add(st_mult(2, args.len), 1), sizeof(wchar_t));
> >>         xutftowcs(wargs, args.buf, 2 * args.len + 1);
> >>         strbuf_release(&args);
> >
> > Argh. Let me write "git commit -a" on the inside of my brown paper bag,
> > so that I actually send out the fix sitting in my working tree, not the
> > half-finished thing I ran "git add" on.
>
> Just to make sure that I am not confused, what you wrote below
> matches what I received from you two message upthread.
>
> I am assuming that it is intended that the two messages from you
> have the same patch, and the assignment of ALLOC_ARRAY to wargs was
> a bug Torsten introduced only to his tree when cutting and pasting.
>
> With that assumption, will queue this one (or the original one,
> which to me is the same thing).

Oh, <phew>. I am only half as incompetent as I thought, then.

I didn't even re-check what I sent, and just assumed Torsten was quoting
it exactly (I _did_ write something like that while making the fix, and
assumed I had just failed to properly commit).

So yes, what I sent both times is the right thing. ;)

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Torsten Bögershausen-2

Oh, <phew>. I am only half as incompetent as I thought, then.

I didn't even re-check what I sent, and just assumed Torsten was quoting
it exactly (I _did_ write something like that while making the fix, and
assumed I had just failed to properly commit).

So yes, what I sent both times is the right thing. ;)

-Peff
--

I have no clue how my copy-paste programing could go so bad :-(
Sorry for the confusion, compat/mingw.c compiles now.

----------------

However, suspecting jk/epipe-in-async, I don't know if we can do
something against this warning:

  CC run-command.o
run-command.c: In function 'async_exit':
run-command.c:631:1: warning: 'noreturn' function does return
  }
  ^

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Jeff King
On Tue, Mar 01, 2016 at 06:49:43AM +0100, Torsten Bögershausen wrote:

> However, suspecting jk/epipe-in-async, I don't know if we can do
> something against this warning:
>
>  CC run-command.o
> run-command.c: In function 'async_exit':
> run-command.c:631:1: warning: 'noreturn' function does return
>  }
>  ^

The only thing that function does is call pthread_exit(), which should
also be marked NORETURN. Looks like the one in compat/win32/pthread.h
isn't?

-Peff
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] compat/mingw: brown paper bag fix for 50a6c8e

Johannes Schindelin
Hi Peff,

On Tue, 1 Mar 2016, Jeff King wrote:

> On Tue, Mar 01, 2016 at 06:49:43AM +0100, Torsten Bögershausen wrote:
>
> > However, suspecting jk/epipe-in-async, I don't know if we can do
> > something against this warning:
> >
> >  CC run-command.o run-command.c: In function 'async_exit':
> >  run-command.c:631:1: warning: 'noreturn' function does return } ^
>
> The only thing that function does is call pthread_exit(), which should
> also be marked NORETURN. Looks like the one in compat/win32/pthread.h
> isn't?
Correct. Expect a patch momentarily.

Ciao,
Dscho
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw

Ramsay Jones-2
In reply to this post by Ramsay Jones-2


On 29/02/16 12:32, Ramsay Jones wrote:

>
>
> On 29/02/16 10:40, Torsten Bögershausen wrote:
>> That compiles OK, thanks.
>>
>>
>> Sorry for high-jacking this thread, but while compiling under CYGWIN,
>> found one warning:
>>
>>    LINK git-credential-store.exe
>>     CC daemon.o
>> daemon.c: In function ‘drop_privileges’:
>> daemon.c:1136:15: warning: implicit declaration of function ‘initgroups’ [-Wimplicit-function-declaration]
>>   if (cred && (initgroups(cred->pass->pw_name, cred->gid) ||
>
> Yeah, this has been there for a while - except it depends on which version
> of the header files you have. (Some may not see the warning).
>
> I have 'fixed' this twice before, then updated my installation and
> a change to the system headers broke it again! (The headers are
> currently 'broken'). So, I got tired of fixing it up and have left
> it a while - you never know a new update may fix it! ;-)
>
> [I personally don't use the git daemon on cygwin, so I don't know
> if this a problem in practice.]
BTW, I forgot to mention that I have had a patch in my local repo for
ages which addresses this issue. However, although this patch fixes
the problem for me, with the header files I currently have installed, it
would just as easily _introduce_ this warning for those that don't
currently see it! ;-)

I suppose that I could send a patch which sets NO_INITGROUPS in the
Makefile, until the system header files are fixed. What do you think?

The correct solution is to fix the <grp.h> header file. I have been
a bit reluctant to tackle that, because I'm not familiar with the
cygwin project development process. Since Newlib is an upstream
project to cygwin, should I go there first/instead?

Anyway, I had a quick squint at the header and I think it needs to
be changed something like the diff below. (I've also attached the
new grp.h file).

[Note: I didn't know what to do about _PATH_GROUP, setgrfile(),
group_from_gid() and setgroupent(), so I punted on those!]

Now, If only we knew someone who could try introducing such a fix
to the cygwin project ...

ATB,
Ramsay Jones
-- >8 --
diff --git a/usr/include/grp.h b/grp.h
index c3a5a67..f2f1902 100644
--- a/usr/include/grp.h
+++ b/grp.h
@@ -67,25 +67,27 @@ extern "C" {
 #ifndef __INSIDE_CYGWIN__
 struct group *getgrgid (gid_t);
 struct group *getgrnam (const char *);
+#if defined __POSIX_VISIBLE || defined __BSD_VISIBLE
 int getgrnam_r (const char *, struct group *,
  char *, size_t, struct group **);
 int getgrgid_r (gid_t, struct group *,
  char *, size_t, struct group **);
-#ifndef _POSIX_SOURCE
-struct group *getgrent (void);
+#endif
+#if defined __BSD_VISIBLE || defined _XOPEN_SOURCE_EXTENDED
 void setgrent (void);
+#if __XSI_VISIBLE >= 700
+struct group *getgrent (void);
 void endgrent (void);
+#endif
+#endif
+#if defined __BSD_VISIBLE
+int initgroups (const char *, gid_t);
+#endif
 #ifndef __CYGWIN__
-void setgrfile (const char *);
-#endif /* !__CYGWIN__ */
-#ifndef _XOPEN_SOURCE
-#ifndef __CYGWIN__
+void setgrfile (const char *); /* 4.3BSD-Reno & Minix */
 char *group_from_gid (gid_t, int);
-int setgroupent (int);
+int setgroupent (int); /* 4.3BSD-Reno & Mac OS X */
 #endif /* !__CYGWIN__ */
-int initgroups (const char *, gid_t);
-#endif /* !_XOPEN_SOURCE */
-#endif /* !_POSIX_SOURCE */
 #endif /* !__INSIDE_CYGWIN__ */
 
 #ifdef __cplusplus
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw

Ramsay Jones-2


On 03/03/16 03:33, Ramsay Jones wrote:

>
>
> On 29/02/16 12:32, Ramsay Jones wrote:
>>
>>
>> On 29/02/16 10:40, Torsten Bögershausen wrote:
>>> That compiles OK, thanks.
>>>
>>>
>>> Sorry for high-jacking this thread, but while compiling under CYGWIN,
>>> found one warning:
>>>
>>>    LINK git-credential-store.exe
>>>     CC daemon.o
>>> daemon.c: In function ‘drop_privileges’:
>>> daemon.c:1136:15: warning: implicit declaration of function ‘initgroups’ [-Wimplicit-function-declaration]
>>>   if (cred && (initgroups(cred->pass->pw_name, cred->gid) ||
>>
>> Yeah, this has been there for a while - except it depends on which version
>> of the header files you have. (Some may not see the warning).
>>
>> I have 'fixed' this twice before, then updated my installation and
>> a change to the system headers broke it again! (The headers are
>> currently 'broken'). So, I got tired of fixing it up and have left
>> it a while - you never know a new update may fix it! ;-)
>>
>> [I personally don't use the git daemon on cygwin, so I don't know
>> if this a problem in practice.]
>
> BTW, I forgot to mention that I have had a patch in my local repo for
> ages which addresses this issue. However, although this patch fixes
> the problem for me, with the header files I currently have installed, it
> would just as easily _introduce_ this warning for those that don't
> currently see it! ;-)
>
> I suppose that I could send a patch which sets NO_INITGROUPS in the
> Makefile, until the system header files are fixed. What do you think?
>
> The correct solution is to fix the <grp.h> header file. I have been
> a bit reluctant to tackle that, because I'm not familiar with the
> cygwin project development process. Since Newlib is an upstream
> project to cygwin, should I go there first/instead?
>
> Anyway, I had a quick squint at the header and I think it needs to
> be changed something like the diff below. (I've also attached the
> new grp.h file).
>
And, of course, I made a mess of it! It was only supposed to be a
'something like this' patch, but still ... :-D

> [Note: I didn't know what to do about _PATH_GROUP, setgrfile(),
> group_from_gid() and setgroupent(), so I punted on those!]

Also, the comments on setgrfile() and setgroupent() were just notes
to myself, which I didn't intend to send ...

So, more like the diff below.

ATB,
Ramsay Jones

-- >8 --
diff --git a/usr/include/grp.h b/grp.h
index c3a5a67..c04fd63 100644
--- a/usr/include/grp.h
+++ b/grp.h
@@ -67,25 +67,25 @@ extern "C" {
 #ifndef __INSIDE_CYGWIN__
 struct group *getgrgid (gid_t);
 struct group *getgrnam (const char *);
+#if defined __POSIX_VISIBLE || defined __BSD_VISIBLE
 int getgrnam_r (const char *, struct group *,
  char *, size_t, struct group **);
 int getgrgid_r (gid_t, struct group *,
  char *, size_t, struct group **);
-#ifndef _POSIX_SOURCE
-struct group *getgrent (void);
+#endif
+#if defined __BSD_VISIBLE || defined __XSI_VISIBLE
 void setgrent (void);
+struct group *getgrent (void);
 void endgrent (void);
+#endif
+#if defined __BSD_VISIBLE
+int initgroups (const char *, gid_t);
+#endif
 #ifndef __CYGWIN__
 void setgrfile (const char *);
-#endif /* !__CYGWIN__ */
-#ifndef _XOPEN_SOURCE
-#ifndef __CYGWIN__
 char *group_from_gid (gid_t, int);
 int setgroupent (int);
 #endif /* !__CYGWIN__ */
-int initgroups (const char *, gid_t);
-#endif /* !_XOPEN_SOURCE */
-#endif /* !_POSIX_SOURCE */
 #endif /* !__INSIDE_CYGWIN__ */
 
 #ifdef __cplusplus
Reply | Threaded
Open this post in threaded view
|

Re: Compiler warning under cygwin/mingw

Junio C Hamano
In reply to this post by SZEDER Gábor
SZEDER Gábor <[hidden email]> writes:

> Hi,
>
>> > And MINGW is not happy for other reasons:
>> >
>> > builtin/rev-parse.c: In function 'cmd_rev_parse':
>> > builtin/rev-parse.c:775:12: warning: implicit declaration of function
>> > 'realpath' [-Wimplicit-function-declaration]
>> >        if (!realpath(gitdir, absolute_path))
>> >             ^
>>
>> I guess you're building "pu"; that is only in sg/completion-updates. I
>> don't know if our custom real_path() would suffice there. You might want
>> to ping the author. The patch is:
>>
>>   http://article.gmane.org/gmane.comp.version-control.git/287462
>
> Oh, I was not aware that there is a custom real_path() that is
> preferred over the system realpath().  I don't see why our real_path()
> would not suffice, it even makes the code a tad shorter.
>
> I will include the patch below in the reroll.

A friendly ping to see if I am missing new development that followed
this message...

>
> Best,
> Gábor
>
>
>  ----  >8  ----
> Subject: [PATCH] fixup! rev-parse: add '--absolute-git-dir' option
>
> ---
>  builtin/rev-parse.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
> index 90a4dd6032c0..d6d9a82c77c4 100644
> --- a/builtin/rev-parse.c
> +++ b/builtin/rev-parse.c
> @@ -762,10 +762,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>   if (!gitdir && !prefix)
>   gitdir = ".git";
>   if (gitdir) {
> - char absolute_path[PATH_MAX];
> - if (!realpath(gitdir, absolute_path))
> - die_errno(_("unable to get absolute path"));
> - puts(absolute_path);
> + puts(real_path(gitdir));
>   continue;
>   }
>   }
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 0/6] modernize t1500

Junio C Hamano
In reply to this post by SZEDER Gábor
Eric Sunshine <[hidden email]> writes:

>   t1500: test_rev_parse: facilitate future test enhancements
>   t1500: reduce dependence upon global state
>   t1500: avoid changing working directory outside of tests
>   t1500: avoid setting configuration options outside of tests
>   t1500: avoid setting environment variables outside of tests
>   t1500: be considerate to future potential tests

When you reroll sg/completion-updates series (87a213f^..2be685a, 21
patches), please pay attention to this series, as it changes the way
you would check the output from your "--absolute-git-dir" option.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html