public inbox for libstdc++@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [PATCH] libstdc++: Introduce GNU/Hurd-specific libstdc++ os-defines.h
       [not found] <20220829003040.kfgr7epzzyavbsjm@begin>
@ 2022-10-07 11:14 ` Jonathan Wakely
  2022-10-07 20:43   ` Samuel Thibault
  0 siblings, 1 reply; 2+ messages in thread
From: Jonathan Wakely @ 2022-10-07 11:14 UTC (permalink / raw)
  To: gcc-patches, bug-hurd; +Cc: libstdc++

As documented at https://gcc.gnu.org/lists.html all patches for
libstdc++ need to be CC'd to the libstdc++ list. That's why your patch
has not been reviewed, because I didn't see it.

On 29/08/22 02:30 +0200, Samuel Thibault wrote:
>This is notably needed because in glibc 2.34, the move of pthread functions
>into libc.so happened for Linux only, not GNU/Hurd.
>
>The pthread_self() function can also always be used fine as it is.
>
>libstdc++-v3/ChangeLog:

>
>        * config/os/gnu/os_defines.h: New file.
>        * config/os/gnu/ctype_base.h: New file.
>        * config/os/gnu/ctype_configure_char.cc: New file.
>        * config/os/gnu/ctype_inline.h: New file.
>        * configure.host: On gnu* host, use os/gnu instead of os/gnu-linux.

I think the commit message should note that these files are all copied
from the existing gnu-linux ones, so that the git log records that and
people don't need to compare them manually.

But if these files are all identical to the existing gnu-linux ones,
except for a couple of macros in os_defines.h being conditional on
Linux, why not just test __linux__ in os_defines.h?

Do we need three new ctype files that are identical to existing files?
Why maintain two copies of the same code?

>diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
>index ba5939d9003..dd288cce2ca 100644
>--- a/libstdc++-v3/ChangeLog
>+++ b/libstdc++-v3/ChangeLog
>@@ -1,3 +1,11 @@
>+2022-08-28  Samuel Thibault  <samuel.thibault@ens-lyon.org>
>+
>+	* config/os/gnu/os_defines.h: New file.
>+	* config/os/gnu/ctype_base.h: New file.
>+	* config/os/gnu/ctype_configure_char.cc: New file.
>+	* config/os/gnu/ctype_inline.h: New file.
>+	* configure.host: On gnu* host, use os/gnu instead of os/gnu-linux.
>+

Please do not include ChangeLog diffs in the patch, it just means the
patch can't be applied cleanly. GCC changelogs are auto-generated
every night. The ChangeLog entry should be submitted as plain text not
inthe diff, or just make it part of your Git commit message.
https://gcc.gnu.org/contribute.html#patches



^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] libstdc++: Introduce GNU/Hurd-specific libstdc++ os-defines.h
  2022-10-07 11:14 ` [PATCH] libstdc++: Introduce GNU/Hurd-specific libstdc++ os-defines.h Jonathan Wakely
@ 2022-10-07 20:43   ` Samuel Thibault
  0 siblings, 0 replies; 2+ messages in thread
From: Samuel Thibault @ 2022-10-07 20:43 UTC (permalink / raw)
  To: Jonathan Wakely; +Cc: gcc-patches, bug-hurd, libstdc++

Hello,

Jonathan Wakely, le ven. 07 oct. 2022 12:14:26 +0100, a ecrit:
> As documented at https://gcc.gnu.org/lists.html all patches for
> libstdc++ need to be CC'd to the libstdc++ list. That's why your patch
> has not been reviewed, because I didn't see it.

Ah, sorry, I hadn't noticed that.

> On 29/08/22 02:30 +0200, Samuel Thibault wrote:
> > This is notably needed because in glibc 2.34, the move of pthread functions
> > into libc.so happened for Linux only, not GNU/Hurd.
> > 
> > The pthread_self() function can also always be used fine as it is.
> > 
> > libstdc++-v3/ChangeLog:
> 
> > 
> >        * config/os/gnu/os_defines.h: New file.
> >        * config/os/gnu/ctype_base.h: New file.
> >        * config/os/gnu/ctype_configure_char.cc: New file.
> >        * config/os/gnu/ctype_inline.h: New file.
> >        * configure.host: On gnu* host, use os/gnu instead of os/gnu-linux.
> 
> I think the commit message should note that these files are all copied
> from the existing gnu-linux ones, so that the git log records that and
> people don't need to compare them manually.
> 
> But if these files are all identical to the existing gnu-linux ones,
> except for a couple of macros in os_defines.h being conditional on
> Linux, why not just test __linux__ in os_defines.h?

Ah, it's just that I hadn't seen that done for other OSes, even when
they are very similar (e.g. between gnu-linux and uclibc).

If #ifdef is fine there, then yes I'm all for it indeed. I'll send a v2.

Samuel

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-10-07 20:43 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <20220829003040.kfgr7epzzyavbsjm@begin>
2022-10-07 11:14 ` [PATCH] libstdc++: Introduce GNU/Hurd-specific libstdc++ os-defines.h Jonathan Wakely
2022-10-07 20:43   ` Samuel Thibault

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).