diff mbox

[hurd,commited] hurd: Fix build ot misc/tst-preadvwritev2-common.c

Message ID 20170903155209.3911-1-samuel.thibault@ens-lyon.org
State Committed, archived
Headers

Commit Message

Samuel Thibault Sept. 3, 2017, 3:52 p.m. UTC 
  * misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
	RWF_NOWAIT): Define to 0 if undefined already.
	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
	boils down to 0.
---
 ChangeLog                       |  4 ++++
 misc/tst-preadvwritev2-common.c | 14 +++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)

Comments

Florian Weimer Sept. 4, 2017, 8:11 a.m. UTC | #1 
On 09/03/2017 05:52 PM, Samuel Thibault wrote:
> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
> +	RWF_NOWAIT): Define to 0 if undefined already.
> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
> +	boils down to 0.

I don't think this is correct.  preadv2 etc. are supposed to be part of
the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
that's a real bug.

Thanks,
Florian
Adhemerval Zanella Sept. 4, 2017, 12:36 p.m. UTC | #2 
On 04/09/2017 05:11, Florian Weimer wrote:
> On 09/03/2017 05:52 PM, Samuel Thibault wrote:
>> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
>> +	RWF_NOWAIT): Define to 0 if undefined already.
>> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
>> +	boils down to 0.
> 
> I don't think this is correct.  preadv2 etc. are supposed to be part of
> the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
> that's a real bug.

I agree and the default implementation on sysdeps/posix/preadv2.c (which
I think it is also the default of hurd) will fail for any flag different
than 0. Should we add a note on manual the current flags are Linux only?
Florian Weimer Sept. 4, 2017, 1:40 p.m. UTC | #3 
On 09/04/2017 02:36 PM, Adhemerval Zanella wrote:
> 
> 
> On 04/09/2017 05:11, Florian Weimer wrote:
>> On 09/03/2017 05:52 PM, Samuel Thibault wrote:
>>> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
>>> +	RWF_NOWAIT): Define to 0 if undefined already.
>>> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
>>> +	boils down to 0.
>>
>> I don't think this is correct.  preadv2 etc. are supposed to be part of
>> the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
>> that's a real bug.
> 
> I agree and the default implementation on sysdeps/posix/preadv2.c (which
> I think it is also the default of hurd) will fail for any flag different
> than 0. Should we add a note on manual the current flags are Linux only?

I would have expected that we'd add the flags to the generic API.

Florian
Adhemerval Zanella Sept. 4, 2017, 2:04 p.m. UTC | #4 
On 04/09/2017 10:40, Florian Weimer wrote:
> On 09/04/2017 02:36 PM, Adhemerval Zanella wrote:
>>
>>
>> On 04/09/2017 05:11, Florian Weimer wrote:
>>> On 09/03/2017 05:52 PM, Samuel Thibault wrote:
>>>> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
>>>> +	RWF_NOWAIT): Define to 0 if undefined already.
>>>> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
>>>> +	boils down to 0.
>>>
>>> I don't think this is correct.  preadv2 etc. are supposed to be part of
>>> the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
>>> that's a real bug.
>>
>> I agree and the default implementation on sysdeps/posix/preadv2.c (which
>> I think it is also the default of hurd) will fail for any flag different
>> than 0. Should we add a note on manual the current flags are Linux only?
> 
> I would have expected that we'd add the flags to the generic API.

I meant that the flags only really *work* on Linux.
Florian Weimer Sept. 4, 2017, 2:06 p.m. UTC | #5 
On 09/04/2017 04:04 PM, Adhemerval Zanella wrote:
> 
> 
> On 04/09/2017 10:40, Florian Weimer wrote:
>> On 09/04/2017 02:36 PM, Adhemerval Zanella wrote:
>>>
>>>
>>> On 04/09/2017 05:11, Florian Weimer wrote:
>>>> On 09/03/2017 05:52 PM, Samuel Thibault wrote:
>>>>> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
>>>>> +	RWF_NOWAIT): Define to 0 if undefined already.
>>>>> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
>>>>> +	boils down to 0.
>>>>
>>>> I don't think this is correct.  preadv2 etc. are supposed to be part of
>>>> the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
>>>> that's a real bug.
>>>
>>> I agree and the default implementation on sysdeps/posix/preadv2.c (which
>>> I think it is also the default of hurd) will fail for any flag different
>>> than 0. Should we add a note on manual the current flags are Linux only?
>>
>> I would have expected that we'd add the flags to the generic API.
> 
> I meant that the flags only really *work* on Linux.

Yes, I think that would be fine.

Florian
Adhemerval Zanella Sept. 4, 2017, 2:07 p.m. UTC | #6 
On 04/09/2017 11:06, Florian Weimer wrote:
> On 09/04/2017 04:04 PM, Adhemerval Zanella wrote:
>>
>>
>> On 04/09/2017 10:40, Florian Weimer wrote:
>>> On 09/04/2017 02:36 PM, Adhemerval Zanella wrote:
>>>>
>>>>
>>>> On 04/09/2017 05:11, Florian Weimer wrote:
>>>>> On 09/03/2017 05:52 PM, Samuel Thibault wrote:
>>>>>> +	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
>>>>>> +	RWF_NOWAIT): Define to 0 if undefined already.
>>>>>> +	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
>>>>>> +	boils down to 0.
>>>>>
>>>>> I don't think this is correct.  preadv2 etc. are supposed to be part of
>>>>> the GNU API, so if Hurd doesn't have definitions of the WRF_* constants,
>>>>> that's a real bug.
>>>>
>>>> I agree and the default implementation on sysdeps/posix/preadv2.c (which
>>>> I think it is also the default of hurd) will fail for any flag different
>>>> than 0. Should we add a note on manual the current flags are Linux only?
>>>
>>> I would have expected that we'd add the flags to the generic API.
>>
>> I meant that the flags only really *work* on Linux.
> 
> Yes, I think that would be fine.

Ok, I will work on that patch.
diff mbox

Patch

diff --git a/ChangeLog b/ChangeLog
index 4802f7c428..824f19a272 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -31,6 +31,10 @@ 
 	__GI_feraiseexcept.
 	* sysdeps/mach/hurd/i386/Implies: Add mach/hurd/x86.
 	* sysdeps/mach/hurd/x86/dl-sysdep.c: New file.
+	* misc/tst-preadvwritev2-common.c (RWF_HIPRI, RWF_DSYNC, RWF_SYNC,
+	RWF_NOWAIT): Define to 0 if undefined already.
+	(do_test_with_invalid_flags): Set invalid_flag to 2 if RWF_SUPPORTED
+	boils down to 0.
 
 2017-09-01  Joseph Myers  <joseph@codesourcery.com>
 
diff --git a/misc/tst-preadvwritev2-common.c b/misc/tst-preadvwritev2-common.c
index 8abedc14d0..9d4909bdb7 100644
--- a/misc/tst-preadvwritev2-common.c
+++ b/misc/tst-preadvwritev2-common.c
@@ -22,9 +22,21 @@ 
 static void
 do_test_with_invalid_flags (void)
 {
+#ifndef RWF_HIPRI
+# define RWF_HIPRI 0
+#endif
+#ifndef RWF_DSYNC
+# define RWF_DSYNC 0
+#endif
+#ifndef RWF_SYNC
+# define RWF_SYNC 0
+#endif
+#ifndef RWF_NOWAIT
+# define RWF_NOWAIT 0
+#endif
 #define RWF_SUPPORTED	(RWF_HIPRI | RWF_DSYNC | RWF_SYNC | RWF_NOWAIT)
   /* Set the next bit from the mask of all supported flags.  */
-  int invalid_flag = __builtin_clz (RWF_SUPPORTED);
+  int invalid_flag = RWF_SUPPORTED != 0 ? __builtin_clz (RWF_SUPPORTED) : 2;
   invalid_flag = 0x1 << ((sizeof (int) * CHAR_BIT) - invalid_flag);
 
   char buf[32];