date:20111121

CPU usage of nanosleep on Win32 when called repeatedly

2011-11-21 Thread Michael Goffioul

Hi,

In the context of octave, I need to have a loop that sleeps for about
100ms then perform some action. Most of the time, the action to
perform is empty so the loop is mainly sleeping. I don't expect such
loop to be CPU-consuming. To build this loop in octave, I'm using
octave_usleep, which calls gnulib nanosleep. I've noticed that such
loop on Win32 is consuming a significant amount of CPU, about 5-6%,
which is suprizing.

I've built a small example (see below) to compare the CPU usage of
gnulib nanosleep and Win32 Sleep (the delay being 100ms, Sleep is good
enough). When running it, I see a steady CPU usage of 5-6% when using
nanosleep, and 0% when using Win32 Sleep.

Is this something that can be fixed in gnulib? Or is this something I
have to live with?
Note that the main octave developer is not really willing to have a
special case with #ifdef for Win32 in octave's code as this is the
specific reason for octave moving to gnulib.

Thanks,
Michael.


#include 
#include 
#define WIN32_LEAN_AND_MEAN
#include 
#include 

int main (int argc, char **argv)
{
  struct timespec t1, rem;
  int i;

  t1.tv_sec = 0;
  t1.tv_nsec = 1;

  printf ("testing gnulib::nanosleep...\n");
  for (i = 0; i < 100; i++)
nanosleep (&t1, &rem);

  printf ("testing Win32 Sleep...\n");
  for (i = 0; i < 100; i++)
Sleep (100);

  return 0;
}

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

2011-11-21 Thread Jim Meyering

Bruno Haible wrote:
> Eric Blake wrote on 2011-08-17:
>>  gl_FUNC_GETCWD_ABORT_BUG([gl_abort_bug=yes]);;
>>esac
>>
>> -  case $gl_cv_func_getcwd_null,$gl_cv_func_getcwd_path_max,$gl_abort_bug in
>> -  *yes,yes,no) ;;
>> +  case 
>> $gl_cv_func_getcwd_null,$gl_cv_func_getcwd_posix_signature$gl_cv_func_getcwd_path_max,$gl_abort_bug
>>  in
>> +  *yes,yes,yes,no) ;;
>>*)
>>  dnl Full replacement lib/getcwd.c, overrides LGPL replacement.
>>  REPLACE_GETCWD=1;;
>
> This change has the effect of setting REPLACE_GETCWD to 1 on *all*
> platforms. A string with only two commas can never math the pattern
> "*yes,yes,yes,no".

Obviously a typo.

> Such as on glibc 2.11/Linux or MacOS X or FreeBSD 6.4, OpenBSD 4.9, NetBSD 
> 5.1:
>   checking whether getcwd (NULL, 0) allocates memory for result... yes
>   checking for getcwd with POSIX signature... yes
>   checking whether getcwd is declared... yes
>   S["REPLACE_GETCWD"]="1"
>
> I'd like to rectify this. The fix is not just to fix the 'case' statement,
> because on OpenBSD 4.9, NetBSD 5.1 that leads to REPLACE_GETCWD=0 (as 
> intended)
> but then the unit test fails with error code 32, i.e. test_abort_bug() 
> returned 4.
>
> On these two platforms,
>   - The test program from m4/getcwd-abort-bug.m4 fails with exit code 4,
>   - but the macrology around it considers an error code 4 to be OK
> (this is also what the comment says: "This is ok, and expected".

That is true.  Remember that this is a configure-time test.
It is testing the system's getcwd.
Some versions of getcwd do fail in that case, yet we must use them when
possible, to handle names shorter than PATH_MAX with inaccessible parents.
In this case, gnulib's getcwd calls the system's getcwd, and handles
the case in which it fails due to the PATH_MAX limitation.

>   - Then, if getcwd does not get overridden by gnulib, in tests/test-getcwd.c
> the function test_abort_bug() returns 4 and the test fails.

That sounds right, because here, we're testing gnulib's replacement getcwd
function.  With gnulib's getcwd, a return value of 4 from test_abort_bug
does indicate a failure.

>   - Finally, the documentation says that the 'getcwd' module fixes the fact
> that "This function does not handle long file names (greater than 
> PATH_MAX)"
>
> So, what should we do?
>   - Change m4/getcwd-abort-bug.m4 to consider an exit code 4 also to be a
> failure?
>   - Or change tests/test-getcwd.c to consider test_abort_bug() = 4 a success
> and change the documentation to state that getcwd(NULL,0) may not work
> with long file names, even when module 'getcwd' is enabled?

gnulib's getcwd should never make test_abort_bug() return 4.
Do you know how/why that happens?

Re: declare sethostname if unistd.h doesn't

2011-11-21 Thread Simon Josefsson

Ben Walton  writes:

> Excerpts from Simon Josefsson's message of Sun Nov 20 14:04:42 -0500 2011:
>
> Hi Simon,
>
>> Have you tested that it resolves the portability issue?  If so that is
>> great.  My man page says Solaris uses this prototype:
>> 
>>  int sethostname(char *name, int namelen);
>
> I used a modified version of the patch to compile inetutils and got
> to the logprio.h issue, which was past the sethostname issue.
>
>> however possibly int and size_t have the same size on all Solaris 8 and
>> 9 architectures?
>
> You're right that this should be int though.  That's what the solaris
> man page indicates...I grabbed the prototype from the wrong terminal
> it would seem.

My point was that the POSIX prototype uses size_t and Solaris seems to
use int, and as Paul pointed out, on 64-bit Solaris systems just
providing the prototype will break -- there needs to be a wrapper
function or a #define that casts the second parameter, or something
similar.  The POSIX prototype has const as well, so you would get
warnings passing a const string to the Solaris function if there is no
casting or wrapper function.

>> In any case, I support installing this if it does something useful
>> on at least some (Solaris) machine, since the current situation does
>> not appear to work on any (Solaris) machine.
>
> It could be a local modification of the inetutils gnulib code until
> the proper version is implemented?

Sure -- I believe we also agreed that the inetutils hostname tool should
still build if sethostname is missing, just without that functionality.
There needs to be some changes for that as well.

/Simon

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

2011-11-21 Thread Bruno Haible

Hi Jim,

> > This change has the effect of setting REPLACE_GETCWD to 1 on *all*
> > platforms. A string with only two commas can never math the pattern
> > "*yes,yes,yes,no".
> 
> Obviously a typo.

Yes, sure.

> > Such as on glibc 2.11/Linux or MacOS X or FreeBSD 6.4, OpenBSD 4.9, NetBSD 
> > 5.1:
> >   checking whether getcwd (NULL, 0) allocates memory for result... yes
> >   checking for getcwd with POSIX signature... yes
> >   checking whether getcwd is declared... yes
> >   S["REPLACE_GETCWD"]="1"
> >
> > I'd like to rectify this. The fix is not just to fix the 'case' statement,
> > because on OpenBSD 4.9, NetBSD 5.1 that leads to REPLACE_GETCWD=0 (as 
> > intended)
> > but then the unit test fails with error code 32, i.e. test_abort_bug() 
> > returned 4.
> >
> > On these two platforms,
> >   - The test program from m4/getcwd-abort-bug.m4 fails with exit code 4,
> >   - but the macrology around it considers an error code 4 to be OK
> > (this is also what the comment says: "This is ok, and expected".
> 
> That is true.  Remember that this is a configure-time test.
> It is testing the system's getcwd.
> Some versions of getcwd do fail in that case, yet we must use them when
> possible, to handle names shorter than PATH_MAX with inaccessible parents.
> In this case, gnulib's getcwd calls the system's getcwd, and handles
> the case in which it fails due to the PATH_MAX limitation.
> 
> >   - Then, if getcwd does not get overridden by gnulib, in 
> > tests/test-getcwd.c
> > the function test_abort_bug() returns 4 and the test fails.
> 
> That sounds right, because here, we're testing gnulib's replacement getcwd
> function.  With gnulib's getcwd, a return value of 4 from test_abort_bug
> does indicate a failure.
> 
> >   - Finally, the documentation says that the 'getcwd' module fixes the fact
> > that "This function does not handle long file names (greater than 
> > PATH_MAX)"
> >
> > So, what should we do?
> >   - Change m4/getcwd-abort-bug.m4 to consider an exit code 4 also to be a
> > failure?
> >   - Or change tests/test-getcwd.c to consider test_abort_bug() = 4 a success
> > and change the documentation to state that getcwd(NULL,0) may not work
> > with long file names, even when module 'getcwd' is enabled?
> 
> gnulib's getcwd should never make test_abort_bug() return 4.
> Do you know how/why that happens?

You are confusing me a bit now. Let me state the problem again.

If we fix Eric's typo, REPLACE_LGPL will come out as 0 on OpenBSD and NetBSD.
This means, lib/getcwd.c will not get compiled, and the getcwd() function that
coreutils and tests/test-getcwd.c see will be *exactly* the system function.
But this system function makes test_abort_bug() return 4.

Can packages that use gnulib's getcwd() module - including coreutils -
expect that in normal conditions (no EPERM anywhere)
 mkdir ("subdir1", 0700);
 chdir ("subdir1");
 mkdir ("subdir2", 0700);
 chdir ("subdir2");
 ...
 mkdir ("subdir1000", 0700);
 chdir ("subdir1000");
 dir = getcwd (NULL, 0);
produces a non-NULL dir (longer than PATH_MAX) ?
Or do they have to be prepared to a NULL result?

> Some versions of getcwd do fail in that case, yet we must use them when
> possible, to handle names shorter than PATH_MAX with inaccessible parents.
> In this case, gnulib's getcwd calls the system's getcwd, and handles
> the case in which it fails due to the PATH_MAX limitation.

So what you are saying is that the unit test and the documentation are
correct, and only the getcwd*.m4 files need to be changed so as to activate
the replacement in lib/getcwd.c. This replacement will first call the
system's getcwd() function, so as to handle file names shorter than PATH_MAX
with inaccessible parents.

Is that what you mean?

Then the comment in m4/getcwd-abort-bug.m4

  /* If libc has the bug in question, this invocation of getcwd
 results in a failed assertion.  */
  cwd = getcwd (NULL, 0);
  if (cwd == NULL)
fail = 4; /* getcwd failed.  This is ok, and expected.  */

needs to be changed to

  /* If libc has the bug in question, this invocation of getcwd
 results in a failed assertion.  */
  cwd = getcwd (NULL, 0);
  if (cwd == NULL)
fail = 4; /* getcwd failed.  This is sufficient for a POSIX compliant
 getcwd() function, but as an extension to POSIX, gnulib's
 getcwd() wants to provide a non-NULL value in this case.  */

Is that right?

Bruno
-- 
In memoriam Ricardo Flores Magón

Re: Failed test on Solaris 8 Sparc

2011-11-21 Thread Simon Josefsson

Dagobert Michelsen  writes:

> Hi Simon,
>
> Am 21.04.2010 um 08:36 schrieb Simon Josefsson:
>> Dagobert Michelsen  writes:
>>> Am 31.03.2009 um 12:23 schrieb Simon Josefsson:
 Dagobert Michelsen  writes:
> I am getting a test failure on Solaris 8 Sparc w/Sun Studio 11:

 Hi! Thanks for the report.  The problem is actually in a self-test
 from
 gnulib.  Your GNU SASL library build should still be fine, if this is
 the only problem.  poll is only used by the 'gsasl' tool, and it most
 likely doesn't require the behaviour tested for by the gnulib self-
 test.
 So a 'make install' should be safe.

>> Unconnected socket test... passed
>> Connected sockets test... failed (expecting POLLHUP after shutdown)
>> General socket test with fork... failed (expecting POLLHUP after
>> shutdown)
>> FAIL: test-poll

 Does anyone recognize this problem?

 If there is no simple solution, I'll disable the self-test in gsasl.
>>> 
>>> Any news on this? I just tested gsasl 1.4.4 on Solaris 9 w/Sun Studio
>>> 12 and the error still occurs.
>> 
>> The error is a likely a problem in the gnulib self-tests.  If that
>> problem isn't resolved in response to your e-mail within a few weeks,
>> please ping me again and I'll disable the self test since in GNU SASL.
>> 
>> Meanwhile you can patch your local copy to not run this particular test,
>> I believe your GNU SASL installation should be fine if this is the only
>> self test failing.
>
> The error is still present in 1.6.1, what can we do about that?
> Talking to the gnulib maintainers?

I have disabled the test-poll self test on the gsasl 1.6.x branch which
is a maintenance branch.  For the 1.7.x development branch and
subsequent stable 1.8.x branch we'll use the latest gnulib and hopefully
this problem has been resolved -- I'll confirm this once I have pushed
1.6.2 out the door.

/Simon

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

2011-11-21 Thread Jim Meyering

Bruno Haible wrote:
...
>> > So, what should we do?
>> >   - Change m4/getcwd-abort-bug.m4 to consider an exit code 4 also to be a
>> > failure?
>> >   - Or change tests/test-getcwd.c to consider test_abort_bug() = 4 a 
>> > success
>> > and change the documentation to state that getcwd(NULL,0) may not work
>> > with long file names, even when module 'getcwd' is enabled?
>>
>> gnulib's getcwd should never make test_abort_bug() return 4.
>> Do you know how/why that happens?
>
> You are confusing me a bit now. Let me state the problem again.

That code tends to confuse me, too.  It is rather tricky.

> If we fix Eric's typo, REPLACE_LGPL will come out as 0 on OpenBSD and NetBSD.

s/REPLACE_LGPL/REPLACE_GETCWD/

Do you know how that happens, given the corrected code?

  case 
$gl_cv_func_getcwd_null,$gl_cv_func_getcwd_posix_signature,$gl_cv_func_getcwd_path_max,$gl_abort_bug
 in
  *yes,yes,yes,no) ;;
  *)
dnl Full replacement lib/getcwd.c, overrides LGPL replacement.
REPLACE_GETCWD=1;;
  esac

That means these must be true:
  - they don't have the getcwd abort bug.  agreed: it was glibc/ia64-specific
  - they have a POSIX signature for getcwd: likely
  - getcwd (NULL, 0) works as we expect for names shorter than PATH_MAX:
  easy to believe.

The part that is surprising is that they would have
$gl_cv_func_getcwd_path_max = yes.
That would mean that getcwd (NULL, 0) can succeed when called
from a directory whose name is longer than PATH_MAX.

If that were true, then their getcwd would not be failing with
test_abort_bug() returning 4.  A contradiction.

> This means, lib/getcwd.c will not get compiled, and the getcwd() function that
> coreutils and tests/test-getcwd.c see will be *exactly* the system function.
> But this system function makes test_abort_bug() return 4.
>
> Can packages that use gnulib's getcwd() module - including coreutils -
> expect that in normal conditions (no EPERM anywhere)
>  mkdir ("subdir1", 0700);
>  chdir ("subdir1");
>  mkdir ("subdir2", 0700);
>  chdir ("subdir2");
>  ...
>  mkdir ("subdir1000", 0700);
>  chdir ("subdir1000");
>  dir = getcwd (NULL, 0);
> produces a non-NULL dir (longer than PATH_MAX) ?

Yes, assuming that nothing else goes wrong.

> Or do they have to be prepared to a NULL result?

They should be prepared for NULL, due to the possibility
of ENOMEM, EIO, etc.

>> Some versions of getcwd do fail in that case, yet we must use them when
>> possible, to handle names shorter than PATH_MAX with inaccessible parents.
>> In this case, gnulib's getcwd calls the system's getcwd, and handles
>> the case in which it fails due to the PATH_MAX limitation.
>
> So what you are saying is that the unit test and the documentation are
> correct, and only the getcwd*.m4 files need to be changed so as to activate
> the replacement in lib/getcwd.c. This replacement will first call the
> system's getcwd() function, so as to handle file names shorter than PATH_MAX
> with inaccessible parents.
>
> Is that what you mean?

Yes.

> Then the comment in m4/getcwd-abort-bug.m4
>
>   /* If libc has the bug in question, this invocation of getcwd
>  results in a failed assertion.  */
>   cwd = getcwd (NULL, 0);
>   if (cwd == NULL)
> fail = 4; /* getcwd failed.  This is ok, and expected.  */
>
> needs to be changed to
>
>   /* If libc has the bug in question, this invocation of getcwd
>  results in a failed assertion.  */
>   cwd = getcwd (NULL, 0);
>   if (cwd == NULL)
> fail = 4; /* getcwd failed.  This is sufficient for a POSIX compliant
>  getcwd() function, but as an extension to POSIX, gnulib's
>  getcwd() wants to provide a non-NULL value in this case.  */
>
> Is that right?

Clarification will help.
However, I think Eric proposed some POSIX wording change that would
allow gnulib's getcwd to be considered compliant.

gnulib-common config.h snippet's use of _MSC_VER

2011-11-21 Thread Simon Josefsson

All,

The file gnulib-common contains some code that ends up in config.h and
thus gets included into all source files, and it looks like this:

#ifndef _Noreturn
# if (3 <= __GNUC__ || (__GNUC__ == 2 && 8 <= __GNUC_MINOR__) \
  || 0x5110 <= __SUNPRO_C)
#  define _Noreturn __attribute__ ((__noreturn__))
# elif 1200 <= _MSC_VER
#  define _Noreturn __declspec (noreturn)
# else
#  define _Noreturn
# endif
#endif

Now with -Wundef I get an error:

../../config.h:365:16: error: "_MSC_VER" is not defined

I understand that gnulib's code isn't clean against -Wundef in general,
however I believe code that gnulib puts into config.h should meet a
higher standard and be as clean as possible, to allow projects to use
whatever kind of warning parameters it choses.  So how about the patch
below?

/Simon

>From 87b3e50c8d73251e210ff4a45dca45185b7c1da9 Mon Sep 17 00:00:00 2001
From: Simon Josefsson 
Date: Mon, 21 Nov 2011 14:18:24 +0100
Subject: [PATCH 2/2] gnulib-common: Silence warnings against config.h code.

* m4/gnulib-common.m4 (_Noreturn): Check that _MSC_VER is defined
before using it, in code that ends up config.h.
---
 ChangeLog   |5 +
 m4/gnulib-common.m4 |2 +-
 2 files changed, 6 insertions(+), 1 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d421257..ae86be3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2011-11-21  Simon Josefsson  
+
+   * m4/gnulib-common.m4 (_Noreturn): Check that _MSC_VER is defined
+   before using it, in code that ends up config.h.
+
 2011-11-20  Bruno Haible  
 
getcwd: Work around getcwd bug on AIX 5..7.
diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
index 7d83299..8621dec 100644
--- a/m4/gnulib-common.m4
+++ b/m4/gnulib-common.m4
@@ -18,7 +18,7 @@ AC_DEFUN([gl_COMMON_BODY], [
 # if (3 <= __GNUC__ || (__GNUC__ == 2 && 8 <= __GNUC_MINOR__) \
   || 0x5110 <= __SUNPRO_C)
 #  define _Noreturn __attribute__ ((__noreturn__))
-# elif 1200 <= _MSC_VER
+# elif defined _MSC_VER && 1200 <= _MSC_VER
 #  define _Noreturn __declspec (noreturn)
 # else
 #  define _Noreturn
-- 
1.7.2.5

Re: gnulib-common config.h snippet's use of _MSC_VER

2011-11-21 Thread Bruno Haible

Hi Simon,

> I understand that gnulib's code isn't clean against -Wundef in general,
> however I believe code that gnulib puts into config.h should meet a
> higher standard

OK.

> +2011-11-21  Simon Josefsson  
> +
> + * m4/gnulib-common.m4 (_Noreturn): Check that _MSC_VER is defined
> + before using it, in code that ends up config.h.

sed -e 's/ends up/ends up in/'

> diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
> index 7d83299..8621dec 100644
> --- a/m4/gnulib-common.m4
> +++ b/m4/gnulib-common.m4
> @@ -18,7 +18,7 @@ AC_DEFUN([gl_COMMON_BODY], [
>  # if (3 <= __GNUC__ || (__GNUC__ == 2 && 8 <= __GNUC_MINOR__) \
>|| 0x5110 <= __SUNPRO_C)
>  #  define _Noreturn __attribute__ ((__noreturn__))
> -# elif 1200 <= _MSC_VER
> +# elif defined _MSC_VER && 1200 <= _MSC_VER
>  #  define _Noreturn __declspec (noreturn)
>  # else
>  #  define _Noreturn

Looks good. Fine with me.

Bruno
-- 
In memoriam Ricardo Flores Magón

Re: Failed test on Solaris 8 Sparc

2011-11-21 Thread Bruno Haible

Simon Josefsson wrote:
> >> Unconnected socket test... passed
> >> Connected sockets test... failed (expecting POLLHUP after shutdown)
> >> General socket test with fork... failed (expecting POLLHUP after
> >> shutdown)
> >> FAIL: test-poll
> ...
> we'll use the latest gnulib and hopefully this problem has been resolved

No, no one has been working on the 'poll' test for 3 years. I still see this
failure regularly on many platforms.

Bruno
-- 
In memoriam Ricardo Flores Magón

Re: gnulib-common config.h snippet's use of _MSC_VER

2011-11-21 Thread Simon Josefsson

Bruno Haible  writes:

> Hi Simon,
>
>> I understand that gnulib's code isn't clean against -Wundef in general,
>> however I believe code that gnulib puts into config.h should meet a
>> higher standard
>
> OK.
>
>> +2011-11-21  Simon Josefsson  
>> +
>> +* m4/gnulib-common.m4 (_Noreturn): Check that _MSC_VER is defined
>> +before using it, in code that ends up config.h.
>
> sed -e 's/ends up/ends up in/'
>
>> diff --git a/m4/gnulib-common.m4 b/m4/gnulib-common.m4
>> index 7d83299..8621dec 100644
>> --- a/m4/gnulib-common.m4
>> +++ b/m4/gnulib-common.m4
>> @@ -18,7 +18,7 @@ AC_DEFUN([gl_COMMON_BODY], [
>>  # if (3 <= __GNUC__ || (__GNUC__ == 2 && 8 <= __GNUC_MINOR__) \
>>|| 0x5110 <= __SUNPRO_C)
>>  #  define _Noreturn __attribute__ ((__noreturn__))
>> -# elif 1200 <= _MSC_VER
>> +# elif defined _MSC_VER && 1200 <= _MSC_VER
>>  #  define _Noreturn __declspec (noreturn)
>>  # else
>>  #  define _Noreturn
>
> Looks good. Fine with me.

Thanks for review -- pushed with typo fix.

/Simon

Re: grep-2.10 on OSF/1

2011-11-21 Thread Eric Blake

[adding bug-gnulib]

 How about having compare "know" about /dev/null.
 Then it can perform the test -s and warn if the file is nonempty.
 With that, all existing (and there are many) /dev/null-using
 compare uses will benefit.
>>
>> Nice idea.

> Yes.  In fact, c-set was against gnulib's init.sh
> (hence included ChangeLog diffs)
> 

> Here's the proposed gnulib commit.

ACK once you fix the comments.

> 
>>From e636d67f6ff4116312c789d4eec2af53b9cd7cd9 Mon Sep 17 00:00:00 2001
> From: Jim Meyering 
> Date: Mon, 21 Nov 2011 21:50:23 +0100
> Subject: [PATCH] init.sh: work around OSF/1 5.1's mishandling of /dev/null
> 
> * tests/init.sh: Make our compare function slightly more portable.
> Reported by Bruno Haible in
> http://thread.gmane.org/gmane.comp.gnu.grep.bugs/4020
> Much improved by Eric Blake.
> ---
>  ChangeLog |8 
>  tests/init.sh |   47 +--
>  2 files changed, 49 insertions(+), 6 deletions(-)
> 
> diff --git a/ChangeLog b/ChangeLog
> index e775587..0fbcf89 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,3 +1,11 @@
> +2011-11-21  Jim Meyering  
> + Eric Blake  
> +
> + init.sh: work around OSF/1 5.1's mishandling of /dev/null
> + * tests/init.sh: Make our compare function slightly more portable.
> + Reported by Bruno Haible in
> + http://thread.gmane.org/gmane.comp.gnu.grep.bugs/4020
> +
>  2011-11-21  Simon Josefsson  
> 
>   * m4/gnulib-common.m4 (_Noreturn): Check that _MSC_VER is defined
> diff --git a/tests/init.sh b/tests/init.sh
> index c78e5b6..68e1f4f 100644
> --- a/tests/init.sh
> +++ b/tests/init.sh
> @@ -221,11 +221,35 @@ export MALLOC_PERTURB_
>  # a partition, or to undo any other global state changes.
>  cleanup_ () { :; }
> 
> +# Arrange not to let diff or cmp operate on /dev/null,
> +# since on some systems (at least OSF/1 5.1), that doesn't work.
> +# When there are not two arguments, return 2.

Should be:

# When there are not two arguments, or neither argument
# is /dev/null, return 2.

> +# When one argument is /dev/null and the other is not empty,
> +# cat the nonempty file to stderr and return 1.
> +# Otherwise, return 0.
> +compare_dev_null_ ()
> +{
> +  test $# = 2 || return 2
> +
> +  if test "x$1" = x/dev/null; then
> +set dummy "$2" "$1"; shift
> +  fi
> +
> +  test "x$2" = x/dev/null || return 2
> +
> +  test -s "$1" || return 0
> +
> +  cat - "$1" <&2
> +Unexpected contents of $1:
> +EOF
> +  return 1
> +}
> +
>  if diff_out_=`( diff -u "$0" "$0" < /dev/null ) 2>/dev/null`; then
>if test -z "$diff_out_"; then
> -compare () { diff -u "$@"; }
> +compare_ () { diff -u "$@"; }
>else
> -compare ()
> +compare_ ()
>  {
>if diff -u "$@" > diff.out; then
>  # No differences were found, but Solaris 'diff' produces output
> @@ -241,9 +265,9 @@ if diff_out_=`( diff -u "$0" "$0" < /dev/null ) 
> 2>/dev/null`; then
>fi
>  elif diff_out_=`( diff -c "$0" "$0" < /dev/null ) 2>/dev/null`; then
>if test -z "$diff_out_"; then
> -compare () { diff -c "$@"; }
> +compare_ () { diff -c "$@"; }
>else
> -compare ()
> +compare_ ()
>  {
>if diff -c "$@" > diff.out; then
>  # No differences were found, but AIX and HP-UX 'diff' produce output
> @@ -259,11 +283,22 @@ elif diff_out_=`( diff -c "$0" "$0" < /dev/null ) 
> 2>/dev/null`; then
>  }
>fi
>  elif ( cmp --version < /dev/null 2>&1 | grep GNU ) > /dev/null 2>&1; then
> -  compare () { cmp -s "$@"; }
> +  compare_ () { cmp -s "$@"; }
>  else
> -  compare () { cmp "$@"; }
> +  compare_ () { cmp "$@"; }
>  fi
> 
> +# Given compare_dev_null_'s preprocessing, for 0 or 2, defer to compare_.
> +# Otherwise, differences have already been printed, so return 1.

Should be:

# Given compare_dev_null_'s preprocessing, defer to compare_ if 2 or
# more. Otherwise, result is correctly 0, or 1 with error output.

> +compare ()
> +{
> +  compare_dev_null_ "$@"
> +  case $? in
> +0|1) return $?;;
> +*) compare_ "$@";;
> +  esac
> +}
> +
>  # An arbitrary prefix to help distinguish test directories.
>  testdir_prefix_ () { printf gt; }
> 
> --
> 1.7.8.rc2.3.g0911
> 

-- 
Eric Blake   ebl...@redhat.com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature

Re: init.sh compare function

2011-11-21 Thread Bruno Haible

[dropping bug-grep from CC]

> > --- a/tests/init.sh
> > +++ b/tests/init.sh
> > @@ -221,11 +221,35 @@ export MALLOC_PERTURB_
> >  # a partition, or to undo any other global state changes.
> >  cleanup_ () { :; }
> > 
> > +# Arrange not to let diff or cmp operate on /dev/null,
> > +# since on some systems (at least OSF/1 5.1), that doesn't work.

I like the idea, but as a person who often looks at failing tests and
interprets the output I have two comments:

1) I would find it useful if, despite recognizing /dev/null as a special
   case, the output format would stay the same or nearly the same.
   When I see a line "+foo bar" or "-foo bar", possibly preceded by a diff
   hunk, I know that the program produced or did not produce a line
   containing "foo bar". Whereas when I see a line "foo bar" I think of
   output that went to stdout of stderr and which should have been piped
   away.

2) I would also find it useful to mention in comments that compare
   function is meant to be called as in
  compare expected_output actual_output
   and not the opposite.

   Why? Because a "+" indicates something that was added, whereas "-"
   indicates something that was removed or missing. Most often the
   program's actual output is wrong, not the expected output.
   If you call
  compare actual_output expected_output
   then extraneous lines come out as "-line" and omitted ones come out as
   "+line", which is counter-intuitive.
   Whereas if you call
  compare expected_output actual_output
   then extraneous lines come out as "+line" and omitted ones come out as
   "-line".

   Yes I know it would take some work to revisit all coreutils and grep
   tests that use 'compare', but that is not an urgent task.

> > +# When one argument is /dev/null and the other is not empty,
> > +# cat the nonempty file to stderr and return 1.
> > +# Otherwise, return 0.
> > +compare_dev_null_ ()
> > +{
> > +  test $# = 2 || return 2
> > +
> > +  if test "x$1" = x/dev/null; then
> > +set dummy "$2" "$1"; shift
> > +  fi
> > +
> > +  test "x$2" = x/dev/null || return 2
> > +
> > +  test -s "$1" || return 0
> > +
> > +  cat - "$1" <&2
> > +Unexpected contents of $1:
> > +EOF

So here I would emit a fake hunk header and then either
   sed 's/^/+/' "$1"
or
   sed 's/^/-/' "$1"

Bruno
-- 
In memoriam Ricardo Flores Magón

CPU usage of nanosleep on Win32 when called repeatedly

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

Re: declare sethostname if unistd.h doesn't

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

Re: Failed test on Solaris 8 Sparc

Re: [PATCH] getcwd-lgpl: fix m4 to match relaxed test for BSD

gnulib-common config.h snippet's use of _MSC_VER

Re: gnulib-common config.h snippet's use of _MSC_VER

Re: Failed test on Solaris 8 Sparc

Re: gnulib-common config.h snippet's use of _MSC_VER

Re: grep-2.10 on OSF/1

Re: init.sh compare function

12 matches

Site Navigation

Mail list logo

Footer information