Upstreaming the glibc Hurd port
Commit Message
Joseph Myers, le mar. 17 avril 2018 23:02:45 +0000, a ecrit:
> On Wed, 18 Apr 2018, Samuel Thibault wrote:
>
> > The patch below would just introduce bits/types/struct___sched_param.h.
> > and bits/types/struct_sched_param.h for all ports since it's the same.
>
> A bits/types/struct_sched_param.h that does "#define sched_param
> __sched_param" is not appropriate for Linux, because it would change the
> C++ mangling of struct sched_param.
Oh.
So that could be as per below?
(the idea is to keep __sched_param close to sched_param so any future
extension would be done alongside).
Samuel
Comments
On Wed, 18 Apr 2018, Samuel Thibault wrote:
> Joseph Myers, le mar. 17 avril 2018 23:02:45 +0000, a ecrit:
> > On Wed, 18 Apr 2018, Samuel Thibault wrote:
> >
> > > The patch below would just introduce bits/types/struct___sched_param.h.
> > > and bits/types/struct_sched_param.h for all ports since it's the same.
> >
> > A bits/types/struct_sched_param.h that does "#define sched_param
> > __sched_param" is not appropriate for Linux, because it would change the
> > C++ mangling of struct sched_param.
>
> Oh.
>
> So that could be as per below?
That seems plausible (this is not a review of this patch).
On Wed, Apr 18, 2018 at 7:13 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> On Wed, 18 Apr 2018, Samuel Thibault wrote:
>
>> Joseph Myers, le mar. 17 avril 2018 23:02:45 +0000, a ecrit:
>> > On Wed, 18 Apr 2018, Samuel Thibault wrote:
>> >
>> > > The patch below would just introduce bits/types/struct___sched_param.h.
>> > > and bits/types/struct_sched_param.h for all ports since it's the same.
>> >
>> > A bits/types/struct_sched_param.h that does "#define sched_param
>> > __sched_param" is not appropriate for Linux, because it would change the
>> > C++ mangling of struct sched_param.
>>
>> Oh.
>>
>> So that could be as per below?
>
> That seems plausible (this is not a review of this patch).
I have a concern: the types 'struct sched_param' and 'struct
__sched_param' are not compatible, even if their members are identical
(6.2.7p1 explicitly requires the tags to be the same for
compatibility).So if you have a 'struct __sched_param' embedded in
your pthread_attr_t, all of the code that copies between that field
and parameters or variables named 'struct sched_param' must do so
field-by-field or with 'memcpy'. The compiler won't let you do a
straight-up structure assignment, but trying to get around that with
pointer tricks will produce undefined behavior.
zw
Zack Weinberg, le mer. 18 avril 2018 09:54:53 -0400, a ecrit:
> On Wed, Apr 18, 2018 at 7:13 AM, Joseph Myers <joseph@codesourcery.com> wrote:
> > On Wed, 18 Apr 2018, Samuel Thibault wrote:
> >
> >> Joseph Myers, le mar. 17 avril 2018 23:02:45 +0000, a ecrit:
> >> > On Wed, 18 Apr 2018, Samuel Thibault wrote:
> >> >
> >> > > The patch below would just introduce bits/types/struct___sched_param.h.
> >> > > and bits/types/struct_sched_param.h for all ports since it's the same.
> >> >
> >> > A bits/types/struct_sched_param.h that does "#define sched_param
> >> > __sched_param" is not appropriate for Linux, because it would change the
> >> > C++ mangling of struct sched_param.
> >>
> >> Oh.
> >>
> >> So that could be as per below?
> >
> > That seems plausible (this is not a review of this patch).
>
> I have a concern: the types 'struct sched_param' and 'struct
> __sched_param' are not compatible, even if their members are identical
> (6.2.7p1 explicitly requires the tags to be the same for
> compatibility).
Ah, probably that's why bits/sched.h used to define both sched_param and
__sched_param with the __sched_priority field, and #define
sched_priority to __sched_priority. I can do that, it's fine.
Samuel
On Wed, Apr 18, 2018 at 10:03 AM, Samuel Thibault
<samuel.thibault@gnu.org> wrote:
>>
>> I have a concern: the types 'struct sched_param' and 'struct
>> __sched_param' are not compatible, even if their members are identical
>> (6.2.7p1 explicitly requires the tags to be the same for
>> compatibility).
>
> Ah, probably that's why bits/sched.h used to define both sched_param and
> __sched_param with the __sched_priority field, and #define
> sched_priority to __sched_priority. I can do that, it's fine.
This is actually worse than having struct sched_param defined by
sys/types.h, because sched.h is now defining *macro* names that it's
not supposed to. It's far more likely that a program will try to
define a variable named 'sched_priority' than that it will try to
define its own 'struct sched_param'.
Can I ask whether you want to keep exposing the internals of pthread_*
types to applications? If there's a plan to stop doing that - even a
"yeah but it's hard and we have so many other things to do first"
intention - then I would recommend leaving sched_param alone for now
and just XFAILing the conformance test. On the other hand, if you
actively want to keep them exposed and encourage applications to use
them, then there does need to be some kind of workaround.
zw
Zack Weinberg, le mer. 18 avril 2018 14:40:29 -0400, a ecrit:
> On Wed, Apr 18, 2018 at 10:03 AM, Samuel Thibault
> <samuel.thibault@gnu.org> wrote:
> >>
> >> I have a concern: the types 'struct sched_param' and 'struct
> >> __sched_param' are not compatible, even if their members are identical
> >> (6.2.7p1 explicitly requires the tags to be the same for
> >> compatibility).
> >
> > Ah, probably that's why bits/sched.h used to define both sched_param and
> > __sched_param with the __sched_priority field, and #define
> > sched_priority to __sched_priority. I can do that, it's fine.
>
> This is actually worse than having struct sched_param defined by
> sys/types.h, because sched.h is now defining *macro* names that it's
> not supposed to. It's far more likely that a program will try to
> define a variable named 'sched_priority' than that it will try to
> define its own 'struct sched_param'.
Sure. glibc just has been #defining sched_priority for a very long time
already :)
> Can I ask whether you want to keep exposing the internals of pthread_*
> types to applications? If there's a plan to stop doing that - even a
> "yeah but it's hard and we have so many other things to do first"
> intention - then I would recommend leaving sched_param alone for now
> and just XFAILing the conformance test.
That's it indeed.
Samuel
@@ -29,10 +29,6 @@
#define SCHED_FIFO 1
#define SCHED_RR 2
-/* Data structure to describe a process' schedulability. */
-struct sched_param
-{
- int sched_priority;
-};
+#include <bits/types/struct_sched_param.h>
#endif /* bits/sched.h */
new file mode 100644
@@ -0,0 +1,28 @@
+/* Sched parameter structure. Generic version.
+ Copyright (C) 1996-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _BITS_TYPES_STRUCT___SCHED_PARAM
+#define _BITS_TYPES_STRUCT___SCHED_PARAM 1
+
+/* Data structure to describe a process' schedulability. */
+struct __sched_param
+{
+ int __sched_priority;
+};
+
+#endif /* bits/types/struct___sched_param.h */
new file mode 100644
@@ -0,0 +1,28 @@
+/* Sched parameter structure. Generic version.
+ Copyright (C) 1996-2018 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <http://www.gnu.org/licenses/>. */
+
+#ifndef _BITS_TYPES_STRUCT_SCHED_PARAM
+#define _BITS_TYPES_STRUCT_SCHED_PARAM 1
+
+/* Data structure to describe a process' schedulability. */
+struct sched_param
+{
+ int sched_priority;
+};
+
+#endif /* bits/types/struct_sched_param.h */
@@ -31,7 +31,8 @@ headers := sys/utsname.h sys/times.h sys/wait.h sys/types.h unistd.h \
bits/local_lim.h tar.h bits/utsname.h bits/confname.h \
bits/waitflags.h bits/waitstatus.h sys/unistd.h sched.h \
bits/sched.h bits/cpu-set.h re_comp.h wait.h bits/environments.h \
- cpio.h spawn.h bits/unistd.h
+ cpio.h spawn.h bits/unistd.h \
+ bits/types/struct___sched_param.h bits/types/struct_sched_param.h
routines := \
uname \
@@ -19,7 +19,7 @@
#ifndef _BITS_TYPES_STRUCT___PTHREAD_ATTR
#define _BITS_TYPES_STRUCT___PTHREAD_ATTR 1
-#include <sched.h>
+#include <bits/types/struct___sched_param.h>
#define __need_size_t
#include <stddef.h>
@@ -32,7 +32,7 @@ enum __pthread_contentionscope;
that not all of them are supported on all systems. */
struct __pthread_attr
{
- struct sched_param __schedparam;
+ struct __sched_param __schedparam;
void *__stackaddr;
size_t __stacksize;
size_t __guardsize;
@@ -24,7 +24,7 @@
#include <pt-internal.h>
const struct __pthread_attr __pthread_default_attr = {
- __schedparam: { sched_priority: 0 },
+ __schedparam: { __sched_priority: 0 },
__stacksize: 0,
__stackaddr: NULL,
#ifdef PAGESIZE
@@ -32,8 +32,8 @@ thread_attr_compare (const pthread_attr_t * left, const pthread_attr_t * right)
struct __pthread_attr *ileft = (struct __pthread_attr *) left;
struct __pthread_attr *iright = (struct __pthread_attr *) right;
- return ileft->__schedparam.sched_priority
- == iright->__schedparam.sched_priority
+ return ileft->__schedparam.__sched_priority
+ == iright->__schedparam.__sched_priority
&& ileft->__stackaddr == iright->__stackaddr
&& ileft->__stacksize == iright->__stacksize
&& ileft->__guardsize == iright->__guardsize
@@ -71,11 +71,7 @@
# define CLONE_IO 0x80000000 /* Clone I/O context. */
#endif
-/* Data structure to describe a process' schedulability. */
-struct sched_param
-{
- int sched_priority;
-};
+#include <bits/types/struct_sched_param.h>
__BEGIN_DECLS