[PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Matt Turner]

Firstly, commit fa1b9bae63 tried to fix the value of SOCK_CLOEXEC,
but missed a trailing 0.

Secondly, the Changelog.hppa says the fix is from Kyle McMartin,
but git thinks the author is Carlos. I definitely think these two
should always match. Use git commit --author=... next time.

From bea02fbe52724594b3b5f87035fbd7d3074a2e5e Mon Sep 17 00:00:00 2001
From: Matt Turner <mattst88@gmail.com>
Date: Fri, 12 Feb 2010 23:39:43 -0500
Subject: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC.

The kernel defines SOCK_CLOEXEC as
include/linux/net.h:#define SOCK_CLOEXEC        O_CLOEXEC
arch/parisc/include/asm/fcntl.h:#define O_CLOEXEC   010000000

But glibc was defining
sysdeps/unix/sysv/linux/hppa/bits/socket.h:  SOCK_CLOEXEC = 01000000,

Signed-off-by: Matt Turner <mattst88@gmail.com>
CC: Carlos O'Donell <carlos@codesourcery.com>
CC: Kyle McMartin <kyle@redhat.com>
---
 ChangeLog.hppa                             |    5 +++++
 sysdeps/unix/sysv/linux/hppa/bits/socket.h |    2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/ChangeLog.hppa b/ChangeLog.hppa
index 28a3c44..228e719 100644
--- a/ChangeLog.hppa
+++ b/ChangeLog.hppa
@@ -1,3 +1,8 @@
+2010-02-12  Matt Turner  <mattst88@gmail.com>
+
+	* sysdeps/unix/sysv/linux/hppa/bits/socket.h: Actually fix
+	SOCK_CLOEXEC to match O_CLOEXEC.
+
 2010-02-01  Kyle McMartin  <kyle@redhat.com>
 
 	* sysdeps/unix/sysv/linux/hppa/bits/socket.h: Fix value of
diff --git a/sysdeps/unix/sysv/linux/hppa/bits/socket.h b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
index bcc27cc..819b398 100644
--- a/sysdeps/unix/sysv/linux/hppa/bits/socket.h
+++ b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
@@ -62,7 +62,7 @@ enum __socket_type
   /* Flags to be ORed into the type parameter of socket and socketpair and
      used for the flags parameter of paccept.  */
 
-  SOCK_CLOEXEC = 01000000,	/* Atomically set close-on-exec flag for the
+  SOCK_CLOEXEC = 010000000,	/* Atomically set close-on-exec flag for the
 				   new descriptor(s).  */
 #define SOCK_CLOEXEC SOCK_CLOEXEC
 #undef SOCK_NONBLOCK
-- 
1.6.4.4

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Carlos O'Donell]

On Fri, Feb 12, 2010 at 11:54 PM, Matt Turner <mattst88@gmail.com> wrote:
> Firstly, commit fa1b9bae63 tried to fix the value of SOCK_CLOEXEC,
> but missed a trailing 0.

Thanks Matt!

> Secondly, the Changelog.hppa says the fix is from Kyle McMartin,
> but git thinks the author is Carlos. I definitely think these two
> should always match. Use git commit --author=... next time.

That's true, I checked in the patch for Kyle. Is this what other
projects do when using GIT to checkin patches for others?

The GNU C Library project is new to GIT so we don't have much procedure.

Would you mind sending an email to libc-alpha@sourceware.org
suggesting this change in procedure?

Once we get an "OK" from the core developers I'll adjust the GIT wiki
instructions for glibc (http://sourceware.org/glibc/wiki/GlibcGit).

> From bea02fbe52724594b3b5f87035fbd7d3074a2e5e Mon Sep 17 00:00:00 2001
> From: Matt Turner <mattst88@gmail.com>
> Date: Fri, 12 Feb 2010 23:39:43 -0500
> Subject: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC.
>
> The kernel defines SOCK_CLOEXEC as
> include/linux/net.h:#define SOCK_CLOEXEC        O_CLOEXEC
> arch/parisc/include/asm/fcntl.h:#define O_CLOEXEC   010000000
>
> But glibc was defining
> sysdeps/unix/sysv/linux/hppa/bits/socket.h:  SOCK_CLOEXEC = 01000000,
>
> Signed-off-by: Matt Turner <mattst88@gmail.com>
> CC: Carlos O'Donell <carlos@codesourcery.com>
> CC: Kyle McMartin <kyle@redhat.com>
> ---
>  ChangeLog.hppa                             |    5 +++++
>  sysdeps/unix/sysv/linux/hppa/bits/socket.h |    2 +-
>  2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/ChangeLog.hppa b/ChangeLog.hppa
> index 28a3c44..228e719 100644
> --- a/ChangeLog.hppa
> +++ b/ChangeLog.hppa
> @@ -1,3 +1,8 @@
> +2010-02-12  Matt Turner  <mattst88@gmail.com>
> +
> +       * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Actually fix
> +       SOCK_CLOEXEC to match O_CLOEXEC.
> +

Thanks for the patch!

>  2010-02-01  Kyle McMartin  <kyle@redhat.com>
>
>        * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Fix value of
> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/socket.h b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
> index bcc27cc..819b398 100644
> --- a/sysdeps/unix/sysv/linux/hppa/bits/socket.h
> +++ b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
> @@ -62,7 +62,7 @@ enum __socket_type
>   /* Flags to be ORed into the type parameter of socket and socketpair and
>      used for the flags parameter of paccept.  */
>
> -  SOCK_CLOEXEC = 01000000,     /* Atomically set close-on-exec flag for the
> +  SOCK_CLOEXEC = 010000000,    /* Atomically set close-on-exec flag for the
>                                   new descriptor(s).  */
>  #define SOCK_CLOEXEC SOCK_CLOEXEC
>  #undef SOCK_NONBLOCK

Kyle, please comment?

Cheers,
Carlos.

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Matt Turner]

On Tue, Feb 16, 2010 at 8:46 AM, Carlos O'Donell
<carlos@systemhalted.org> wrote:
> On Fri, Feb 12, 2010 at 11:54 PM, Matt Turner <mattst88@gmail.com> wrote:
>> Firstly, commit fa1b9bae63 tried to fix the value of SOCK_CLOEXEC,
>> but missed a trailing 0.
>
> Thanks Matt!
>
>> Secondly, the Changelog.hppa says the fix is from Kyle McMartin,
>> but git thinks the author is Carlos. I definitely think these two
>> should always match. Use git commit --author=... next time.
>
> That's true, I checked in the patch for Kyle. Is this what other
> projects do when using GIT to checkin patches for others?

Yeah, this is pretty standard procedure and is used in the couple of
projects I'm involved with, namely the Linux kernel and Xorg and
friends.

> The GNU C Library project is new to GIT so we don't have much procedure.
>
> Would you mind sending an email to libc-alpha@sourceware.org
> suggesting this change in procedure?

Do other people not do this? Looks like all my patches that Joseph has
committed are attributed to me in git. I don't see anything committed
to glibc.git by anyone other than Ulrich Drepper, but plenty of
authors.

Matt

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 594 bytes --]

On Tuesday 16 February 2010 08:46:46 Carlos O'Donell wrote:
> On Fri, Feb 12, 2010 at 11:54 PM, Matt Turner <mattst88@gmail.com> wrote:
> > Secondly, the Changelog.hppa says the fix is from Kyle McMartin,
> > but git thinks the author is Carlos. I definitely think these two
> > should always match. Use git commit --author=... next time.
> 
> That's true, I checked in the patch for Kyle. Is this what other
> projects do when using GIT to checkin patches for others?

use `git am` to apply the e-mail and git will automatically populate the 
author/committer fields for you
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Carlos O'Donell]

On Tue, Feb 16, 2010 at 3:12 PM, Matt Turner <mattst88@gmail.com> wrote:
> Do other people not do this? Looks like all my patches that Joseph has
> committed are attributed to me in git. I don't see anything committed
> to glibc.git by anyone other than Ulrich Drepper, but plenty of
> authors.

It sounds like this is common practice then. Thanks.

I'll document this on the glibc wiki.

What ways other than `git am` and `git commit --author=` are there to
indicate the true author of the patch?

Cheers,
Carlos.

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Carlos O'Donell]

On Tue, Feb 16, 2010 at 8:46 AM, Carlos O'Donell
<carlos@systemhalted.org> wrote:
>> --- a/ChangeLog.hppa
>> +++ b/ChangeLog.hppa
>> @@ -1,3 +1,8 @@
>> +2010-02-12  Matt Turner  <mattst88@gmail.com>
>> +
>> +       * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Actually fix
>> +       SOCK_CLOEXEC to match O_CLOEXEC.
>> +
>
> Thanks for the patch!
>
>>  2010-02-01  Kyle McMartin  <kyle@redhat.com>
>>
>>        * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Fix value of
>> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/socket.h b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>> index bcc27cc..819b398 100644
>> --- a/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>> +++ b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>> @@ -62,7 +62,7 @@ enum __socket_type
>>   /* Flags to be ORed into the type parameter of socket and socketpair and
>>      used for the flags parameter of paccept.  */
>>
>> -  SOCK_CLOEXEC = 01000000,     /* Atomically set close-on-exec flag for the
>> +  SOCK_CLOEXEC = 010000000,    /* Atomically set close-on-exec flag for the
>>                                   new descriptor(s).  */
>>  #define SOCK_CLOEXEC SOCK_CLOEXEC
>>  #undef SOCK_NONBLOCK
>
> Kyle, please comment?

Kyle responded to me privately indicating that your patch is correct.

Applied.

Thanks Matt, please check the git commit to see if everything was in order.

Cheers,
Carlos.

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Matt Turner]

On Wed, Feb 17, 2010 at 10:16 AM, Carlos O'Donell
<carlos@systemhalted.org> wrote:
> On Tue, Feb 16, 2010 at 8:46 AM, Carlos O'Donell
> <carlos@systemhalted.org> wrote:
>>> --- a/ChangeLog.hppa
>>> +++ b/ChangeLog.hppa
>>> @@ -1,3 +1,8 @@
>>> +2010-02-12  Matt Turner  <mattst88@gmail.com>
>>> +
>>> +       * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Actually fix
>>> +       SOCK_CLOEXEC to match O_CLOEXEC.
>>> +
>>
>> Thanks for the patch!
>>
>>>  2010-02-01  Kyle McMartin  <kyle@redhat.com>
>>>
>>>        * sysdeps/unix/sysv/linux/hppa/bits/socket.h: Fix value of
>>> diff --git a/sysdeps/unix/sysv/linux/hppa/bits/socket.h b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>>> index bcc27cc..819b398 100644
>>> --- a/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>>> +++ b/sysdeps/unix/sysv/linux/hppa/bits/socket.h
>>> @@ -62,7 +62,7 @@ enum __socket_type
>>>   /* Flags to be ORed into the type parameter of socket and socketpair and
>>>      used for the flags parameter of paccept.  */
>>>
>>> -  SOCK_CLOEXEC = 01000000,     /* Atomically set close-on-exec flag for the
>>> +  SOCK_CLOEXEC = 010000000,    /* Atomically set close-on-exec flag for the
>>>                                   new descriptor(s).  */
>>>  #define SOCK_CLOEXEC SOCK_CLOEXEC
>>>  #undef SOCK_NONBLOCK
>>
>> Kyle, please comment?
>
> Kyle responded to me privately indicating that your patch is correct.
>
> Applied.
>
> Thanks Matt, please check the git commit to see if everything was in order.
>
> Cheers,
> Carlos.
>

Looks perfect. Thanks!
Matt

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Carlos O'Donell]

On Wed, Feb 17, 2010 at 9:23 AM, Carlos O'Donell
<carlos@systemhalted.org> wrote:
> On Tue, Feb 16, 2010 at 3:12 PM, Matt Turner <mattst88@gmail.com> wrote:
>> Do other people not do this? Looks like all my patches that Joseph has
>> committed are attributed to me in git. I don't see anything committed
>> to glibc.git by anyone other than Ulrich Drepper, but plenty of
>> authors.
>
> It sounds like this is common practice then. Thanks.
>
> I'll document this on the glibc wiki.
>
> What ways other than `git am` and `git commit --author=` are there to
> indicate the true author of the patch?

I've updated the glibc wiki to mention these two things under
Wiki->Glibc Git->Git Help for Commiters->Commit Messages section.

Cheers,
Carlos.

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Roland McGrath]

> What ways other than `git am` and `git commit --author=` are there to
> indicate the true author of the patch?

I think you can do it with a magic "comment" when editting the log message.

Re: [PATCH] Fix SOCK_CLOEXEC on HPPA to match O_CLOEXEC

[Jim Meyering]

Carlos O'Donell wrote:
> On Tue, Feb 16, 2010 at 3:12 PM, Matt Turner <mattst88@gmail.com> wrote:
>> Do other people not do this? Looks like all my patches that Joseph has
>> committed are attributed to me in git. I don't see anything committed
>> to glibc.git by anyone other than Ulrich Drepper, but plenty of
>> authors.
>
> It sounds like this is common practice then. Thanks.
>
> I'll document this on the glibc wiki.
>
> What ways other than `git am` and `git commit --author=` are there to
> indicate the true author of the patch?

If you commit it locally with yourself as the author
when in fact someone else should be listed as the author,
you can change it with this:

    git commit --amend --author='Some Other User <user@example.com>'

Here are a few personal guidelines I follow in projects I own:

Even if I make some (but not "sweeping") changes, I generally leave the
original author's name as commit "Author".  However, when amending it's
important to be very sure of what you're doing, and usually good to get
sign-off from the contributor.  It's not pleasant to "adjust" a patch
incorrectly and then to commit/push the bug (to a non-rewindable branch)
in the name of someone else.

Likewise, on the giving-credit front, when I post a patch for review,
and someone spots a bug in it, I've begun to add an "Improved by: ..."
line to give them credit for that.