[PATCH] Fix p{readv,writev}{64} consolidation implementation

Florian Weimer fweimer@redhat.com
Wed Jun 15 06:50:00 GMT 2016


On 06/15/2016 07:37 AM, Mike Frysinger wrote:

> although i guess this isn't too surprising as this code was in the
> generic sysdeps dir which currently doesn't have as many users as
> we wish it did :).

Yeah, I think we have to rely on the new test to catch any outliers.

>> +#define FAIL() \
>> +  do { printf ("error: failure on line %d\n", __LINE__); return 1; } while (0)
>> ...
>> +  ret = pwritev (temp_fd, iov, 2, 0);
>> +  if (ret == -1)
>> +    FAIL ();
>
> as a personal stance, never been a big fan of messages that just have a
> line number.  when you get reports/logs, you end having to chase down the
> exact same source tree as the reporter.

At least there is a line number.  Many existing tests do not even have that.

> why can't we just assert() everywhere ?  we rely on that in tests already
> and we wouldn't have to do any checking otherwise.
>   ret = pwritev (temp_fd, iov, 2, 0);
>   assert (ret == sizeof buf1 + sizeof buf2);

assert writes to standard error, not standard output, where it will be 
captured.  I don't know why we don't capture both.

There seems to be a policy against tests aborting on failure, too, but 
we are not very consistent about that.  In my experience, people who 
build and test glibc are more likely to shrug off tests with non-zero 
exit status than to ignore tests with aborts, although they really 
should treat them the same.

I think we should really have some sort of libtest.a, which provides 
test helpers, error-checking functions such as xmalloc, and general 
helpers for setting up special test environments.  I'm a bit worried 
about figuring out the proper dependencies, so that libtest.a is built 
before all the tests are linked.

Florian



More information about the Libc-alpha mailing list