cygcheck exit status
As mentioned on cygwin (hopefully I'm not falling afoul of trivial patch size,
since I don't have assignment; and hopefully gmane didn't kill this):
2005-07-05 Eric Blake <[EMAIL PROTECTED]>
* cygcheck.cc (track_down, cygcheck): Return true on success.
(main): Reflect cygcheck failures in exit status.
Index: winsup/utils/cygcheck.cc
===
RCS file: /cvs/src/src/winsup/utils/cygcheck.cc,v
retrieving revision 1.74
diff -u -b -p -r1.74 cygcheck.cc
--- winsup/utils/cygcheck.cc30 May 2005 15:49:31 - 1.74
+++ winsup/utils/cygcheck.cc5 Jul 2005 20:46:38 -
@@ -364,7 +364,7 @@ struct ImpDirectory
};
-static void track_down (char *file, char *suffix, int lvl);
+static bool track_down (char *file, char *suffix, int lvl);
#define CYGPREFIX (sizeof ("%%% Cygwin ") - 1)
static void
@@ -554,26 +554,27 @@ dll_info (const char *path, HANDLE fh, i
cygwin_info (fh);
}
-static void
+// Return true on success, false if error printed
+static bool
track_down (char *file, char *suffix, int lvl)
{
if (file == NULL)
{
display_error ("track_down: NULL passed for file", true, false);
- return;
+ return false;
}
if (suffix == NULL)
{
display_error ("track_down: NULL passed for suffix", false, false);
- return;
+ return false;
}
char *path = find_on_path (file, suffix, 0, 1);
if (!path)
{
printf ("Error: could not find %s\n", file);
- return;
+ return false;
}
Did *d = already_did (file);
@@ -589,7 +590,7 @@ track_down (char *file, char *suffix, in
printf ("%s", path);
printf (" (recursive)\n");
}
- return;
+ return true;
case DID_INACTIVE:
if (verbose)
{
@@ -598,7 +599,7 @@ track_down (char *file, char *suffix, in
printf ("%s", path);
printf (" (already done)\n");
}
- return;
+ return true;
default:
break;
}
@@ -609,7 +610,7 @@ track_down (char *file, char *suffix, in
if (!path)
{
printf ("%s not found\n", file);
- return;
+ return false;
}
printf ("%s", path);
@@ -620,7 +621,7 @@ track_down (char *file, char *suffix, in
if (fh == INVALID_HANDLE_VALUE)
{
printf (" - Cannot open\n");
- return;
+ return false;
}
d->state = DID_ACTIVE;
@@ -629,6 +630,7 @@ track_down (char *file, char *suffix, in
d->state = DID_INACTIVE;
if (!CloseHandle (fh))
display_error ("track_down: CloseHandle()");
+ return true;
}
static void
@@ -653,14 +655,15 @@ ls (char *f)
display_error ("ls: CloseHandle()");
}
-static void
+// Return true on success, false if error printed
+static bool
cygcheck (char *app)
{
char *papp = find_on_path (app, (char *) ".exe", 1, 0);
if (!papp)
{
printf ("Error: could not find %s\n", app);
- return;
+ return false;
}
char *s = strdup (papp);
char *sl = 0, *t;
@@ -675,7 +678,7 @@ cygcheck (char *app)
paths[0] = s;
}
did = 0;
- track_down (papp, (char *) ".exe", 0);
+ return track_down (papp, (char *) ".exe", 0);
}
@@ -1590,6 +1593,7 @@ int
main (int argc, char **argv)
{
int i;
+ bool ok = true;
load_cygwin (argc, argv);
(void) putenv("POSIXLY_CORRECT=1");
@@ -1677,7 +1681,7 @@ main (int argc, char **argv)
{
if (i)
puts ("");
- cygcheck (argv[i]);
+ ok &= cygcheck (argv[i]);
}
if (sysinfo)
@@ -1693,5 +1697,5 @@ main (int argc, char **argv)
puts ("Use -h to see help about each section");
}
- return 0;
+ return ok ? EXIT_SUCCESS : EXIT_FAILURE;
}
Re: cygcheck exit status
On Tue, Jul 05, 2005 at 08:49:06PM +, Eric Blake wrote:
>@@ -1677,7 +1681,7 @@ main (int argc, char **argv)
> {
>if (i)
> puts ("");
>- cygcheck (argv[i]);
>+ ok &= cygcheck (argv[i]);
Why are you anding the result here? Why not just set ok = cygcheck (...)?
cgf
Re: cygcheck exit status
Christopher Faylor cygwin.com> writes:
>
> On Tue, Jul 05, 2005 at 08:49:06PM +, Eric Blake wrote:
> > -1677,7 +1681,7 main (int argc, char **argv)
> > {
> >if (i)
> > puts ("");
> >- cygcheck (argv[i]);
> >+ ok &= cygcheck (argv[i]);
>
> Why are you anding the result here? Why not just set ok = cygcheck (...)?
Because it's in a for loop, and when the first file fails but second succeeds,
you still want the overall command to exit with failure.
Re: cygcheck exit status
On Tue, Jul 05, 2005 at 08:57:42PM +, Eric Blake wrote:
>Christopher Faylor cygwin.com> writes:
>>On Tue, Jul 05, 2005 at 08:49:06PM +, Eric Blake wrote:
>>> -1677,7 +1681,7 main (int argc, char **argv)
>>> {
>>>if (i)
>>> puts ("");
>>>- cygcheck (argv[i]);
>>>+ ok &= cygcheck (argv[i]);
>>
>>Why are you anding the result here? Why not just set ok = cygcheck
>>(...)?
>
>Because it's in a for loop, and when the first file fails but second
>succeeds, you still want the overall command to exit with failure.
Huh. That's two useful things I've learned about C in the span of a
week.
I'll check this in.
Thanks.
cgf
Re: cygcheck exit status
On Tue, 5 Jul 2005, Eric Blake wrote:
> Christopher Faylor cygwin.com> writes:
>
> >
> > On Tue, Jul 05, 2005 at 08:49:06PM +, Eric Blake wrote:
> > > -1677,7 +1681,7 main (int argc, char **argv)
> > > {
> > >if (i)
> > > puts ("");
> > >- cygcheck (argv[i]);
> > >+ ok &= cygcheck (argv[i]);
> >
> > Why are you anding the result here? Why not just set ok = cygcheck (...)?
>
> Because it's in a for loop, and when the first file fails but second
> succeeds, you still want the overall command to exit with failure.
That's the correct intent, but shouldn't it be &&= instead of &=,
technically?
Igor
--
http://cs.nyu.edu/~pechtcha/
|\ _,,,---,,_[EMAIL PROTECTED]
ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED]
|,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D.
'---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow!
"The Sun will pass between the Earth and the Moon tonight for a total
Lunar eclipse..." -- WCBS Radio Newsbrief, Oct 27 2004, 12:01 pm EDT
RE: cygcheck exit status
Original Message
>From: Igor Pechtchanski
>Sent: 06 July 2005 15:02
> On Tue, 5 Jul 2005, Eric Blake wrote:
>
>> Christopher Faylor cygwin.com> writes:
>>
>>>
>>> On Tue, Jul 05, 2005 at 08:49:06PM +, Eric Blake wrote:
-1677,7 +1681,7 main (int argc, char **argv)
{ if (i)
puts ("");
- cygcheck (argv[i]);
+ ok &= cygcheck (argv[i]);
>>>
>>> Why are you anding the result here? Why not just set ok = cygcheck
>>> (...)?
>>
>> Because it's in a for loop, and when the first file fails but second
>> succeeds, you still want the overall command to exit with failure.
>
> That's the correct intent, but shouldn't it be &&= instead of &=,
> technically?
> Igor
Nope, because then it wouldn't evaluate operand (== call the function)
after the first failure.
cheers,
DaveK
--
Can't think of a witty .sigline today
Re: cygcheck exit status
Igor Pechtchanski cs.nyu.edu> writes: > > Because it's in a for loop, and when the first file fails but second > > succeeds, you still want the overall command to exit with failure. > > That's the correct intent, but shouldn't it be &&= instead of &=, > technically? There's no such thing as &&=. And even if there was, you wouldn't want to use it, because it would short-circuit running cygcheck(). The whole point of the boolean collector is to run the test on every file, but to remember if any of the tests failed. Maybe thinking of a short-circuit in the reverse direction will help you understand: ok = cygcheck (argv[i]) && ok; But since ok is a simple boolean, short-circuiting doesn't save us any side effects, so we can use: ok = cygcheck (argv[i]) & ok; And since & is commutative, it has the same outcome as: ok = ok & cygcheck (argv[i]); Hence my shorthand (coreutils uses this idiom a lot, too): ok &= cygcheck (argv[i]); [By deMorgan's law, I could have also reversed the sense of the collector: bool failed = false; for (int i; ...) failed |= test_that_returns_true_on_failure(); return failed ? EXIT_FAILURE : EXIT_SUCCESS; But I hate thinking in negative logic, hence my definition of cygcheck to return true on success.] -- Eric Blake
RE: cygcheck exit status
Original Message >From: Eric Blake >Sent: 06 July 2005 15:19 > > But I hate thinking in negative logic, hence my definition of cygcheck to > return true on success.] Mneh. I don't like boolean success-fail return values full stop. They convey too little information and then need to be supplanted with hideous kludges such as global variables called errno or subsidiary function calls called GetLastError. The standard POSIX design of returning zero for success or a non-zero error code for failure is now something that's so much second nature I don't even think of it as negative logic. However this should not be taken as any opposition to your proposed patch! cheers, DaveK -- Can't think of a witty .sigline today
Re: cygcheck exit status
On Wed, 6 Jul 2005, Eric Blake wrote: > Igor Pechtchanski cs.nyu.edu> writes: > > > Because it's in a for loop, and when the first file fails but second > > > succeeds, you still want the overall command to exit with failure. > > > > That's the correct intent, but shouldn't it be &&= instead of &=, > > technically? > > There's no such thing as &&=. And even if there was, you wouldn't want > to use it, because it would short-circuit running cygcheck(). The whole > point of the boolean collector is to run the test on every file, but to > remember if any of the tests failed. Maybe thinking of a short-circuit > in the reverse direction will help you understand: > [snip] Ok, ok, IOWTWIWT... :-) I'm well aware of the short circuiting behavior of &&. Igor -- http://cs.nyu.edu/~pechtcha/ |\ _,,,---,,_[EMAIL PROTECTED] ZZZzz /,`.-'`'-. ;-;;,_[EMAIL PROTECTED] |,4- ) )-,_. ,\ ( `'-' Igor Pechtchanski, Ph.D. '---''(_/--' `-'\_) fL a.k.a JaguaR-R-R-r-r-r-.-.-. Meow! "The Sun will pass between the Earth and the Moon tonight for a total Lunar eclipse..." -- WCBS Radio Newsbrief, Oct 27 2004, 12:01 pm EDT
RE: cygcheck exit status
Original Message >From: Igor Pechtchanski >Sent: 06 July 2005 16:36 > On Wed, 6 Jul 2005, Eric Blake wrote: > >> Igor Pechtchanski cs.nyu.edu> writes: Because it's in a for loop, and when the first file fails but second succeeds, you still want the overall command to exit with failure. >>> >>> That's the correct intent, but shouldn't it be &&= instead of &=, >>> technically? >> >> There's no such thing as &&=. And even if there was, you wouldn't want >> to use it, because it would short-circuit running cygcheck(). The whole >> point of the boolean collector is to run the test on every file, but to >> remember if any of the tests failed. Maybe thinking of a short-circuit >> in the reverse direction will help you understand: >> [snip] > > Ok, ok, IOWTWIWT... :-) I'm well aware of the short circuiting > behavior of &&. > Igor I thought it too when I first looked at the code, but realised the short-circuit implication before I had time to write a reply But it _was_ news to me that there's no &&= operator! cheers, DaveK -- Can't think of a witty .sigline today
