Discussion:
Run a command and determine if it succeeded
(too old to reply)
Frederick Gotham
2020-09-10 10:17:21 UTC
Permalink
In a C program, I'm using the "system" function to execute a command, for example:

int const status = system("grep sdcard /proc/mounts");

I then want to be check the integer value returned by the program "grep" to see if it succeeded.

On Linux, it's not a simple as:

if ( EXIT_SUCCESS == status )
{
/* code */
}

I'm looking in the Linux reference manual and I see the following for the "system" function:

The return value of system() is one of the following:

* (a) If command is NULL, then a nonzero value if a shell is available,
or 0 if no shell is available.

* (b) If a child process could not be created, or its status could not
be retrieved, the return value is -1 and errno is set to indicate
the error.

* (c) If a shell could not be executed in the child process, then the
return value is as though the child shell terminated by calling
_exit(2) with the status 127.

* (d) If all system calls succeed, then the return value is the
termination status of the child shell used to execute command.
(The termination status of a shell is the termination status of
the last command it executes.)

In the last two cases, the return value is a "wait status" that can
be examined using the macros described in waitpid(2). (i.e.,
WIFEXITED(), WEXITSTATUS(), and so on).

system() does not affect the wait status of any other children.

Of the four scenarios listed above, I only have to consider (b), (c), (d).

For (b), the check is:

if ( -1 != status )

For (c) and (d), the check is:

if ( WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )

And so if I put them all together, I get:

int const status = system("grep sdcard /proc/mounts");

if ( (-1 != status) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}

I wonder if I'm missing something here though, because when I do a web search on how to analyse the return value from "system" on Linux, I see people do this:

if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}

I don't see why they're checking for "less than zero" like that instead of checking for -1. This seems to be quite common on sites like Stack Overflow.
b***@nuttyella.co.uk
2020-09-10 10:29:54 UTC
Permalink
On Thu, 10 Sep 2020 03:17:21 -0700 (PDT)
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
I then want to be check the integer value returned by the program "grep" to
see if it succeeded.
Why would you use system() with grep anyway? You can't capture the output
of grep in the parent process this way so unless you just want it to print to
stdout then there's no point. If you don't want the fuss of fork-exec then
just use popen() & pclose(). popen will allow you to capture greps output and
pclose returns the childs exit status. Unfortunately popen() also execs a
shell first so is less efficient that fork-exec.
Lew Pitcher
2020-09-10 15:27:55 UTC
Permalink
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
I then want to be check the integer value returned by the program "grep"
to see if it succeeded.
[snip]
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
if ( (-1 != status) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I wonder if I'm missing something here though, because when I do a web
search on how to analyse the return value from "system" on Linux, I see
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS ==
WEXITSTATUS(status) ) {
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that instead
of checking for -1.
Probably because system calls externalize error conditions as negative
integers, and, while the documentation may specifically refer to -1 in /
this/ version of the system call, /other/ versions of the system call may
return other negative values for other errors.

And, it's shorter to type.
Post by Frederick Gotham
This seems to be quite common on sites like Stack Overflow.
Which "this"? the "== -1" approach or the "< 0" approach?

Most of the Unix C code that I've seen (or written, for that matter) uses
the "< 0" approach, as a cover-your-a$$ catch of error conditions.

(FWIW, I have my own opinions on the quality of Stack Overflow answers.
Suffice it to say that I /refuse to/ use Stack Overflow.)

HTH
--
Lew Pitcher
"In Skills, We Trust"
Scott Lurndal
2020-09-10 16:06:17 UTC
Permalink
Post by Lew Pitcher
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
I then want to be check the integer value returned by the program "grep"
to see if it succeeded.
[snip]
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
if ( (-1 != status) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I wonder if I'm missing something here though, because when I do a web
search on how to analyse the return value from "system" on Linux, I see
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS ==
WEXITSTATUS(status) ) {
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that instead
of checking for -1.
Probably because system calls externalize error conditions as negative
integers, and, while the documentation may specifically refer to -1 in /
this/ version of the system call, /other/ versions of the system call may
return other negative values for other errors.
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
Keith Thompson
2020-09-10 20:23:06 UTC
Permalink
***@slp53.sl.home (Scott Lurndal) writes:
[...]
Post by Scott Lurndal
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
mmap() returns a void*. Pointer values are neither negative nor
positive. A pointer value might have a representation equivalent to
that of a negative integer, but if you write

if (mmap(...) < 0) // replace ... by valid arguments

your code won't compile.
--
Keith Thompson (The_Other_Keith) Keith.S.Thompson+***@gmail.com
Working, but not speaking, for Philips Healthcare
void Void(void) { Void(); } /* The recursive call of the void */
Kaz Kylheku
2020-09-10 22:56:17 UTC
Permalink
Post by Keith Thompson
[...]
Post by Scott Lurndal
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
mmap() returns a void*. Pointer values are neither negative nor
positive. A pointer value might have a representation equivalent to
that of a negative integer, but if you write
if (mmap(...) < 0) // replace ... by valid arguments
your code won't compile.
With GCC, a popular compiler, I had to use -pedantic to obtain

mmap.c:8:11: warning: ordered comparison of pointer with integer zero [-Wpedantic]
if (mem < 0)
^

The main problem is that it doesn't actually work. The MAP_FAILED value
that is (void *) -1 is not comparing lying below the null pointer.
(void *) -1 looks like a high address.

Anyway, even someone who doesn't care about maximal portability cannot
use this.
--
TXR Programming Lanuage: http://nongnu.org/txr
Music DIY Mailing List: http://www.kylheku.com/diy
ADA MP-1 Mailing List: http://www.kylheku.com/mp1
Keith Thompson
2020-09-11 00:39:36 UTC
Permalink
Post by Kaz Kylheku
Post by Keith Thompson
[...]
Post by Scott Lurndal
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
mmap() returns a void*. Pointer values are neither negative nor
positive. A pointer value might have a representation equivalent to
that of a negative integer, but if you write
if (mmap(...) < 0) // replace ... by valid arguments
your code won't compile.
With GCC, a popular compiler, I had to use -pedantic to obtain
mmap.c:8:11: warning: ordered comparison of pointer with integer zero [-Wpedantic]
if (mem < 0)
^
Right. Rather than "won't compile" I should have written something like
"will be diagnosed by a conforming compiler". (gcc in its
non-conforming default mode permits a lot of things that are constraint
violations. It hadn't occurred to me that that would be one of them.)
Post by Kaz Kylheku
The main problem is that it doesn't actually work. The MAP_FAILED value
that is (void *) -1 is not comparing lying below the null pointer.
(void *) -1 looks like a high address.
Apparently so. A conforming compiler could equally validly use the
equivalent of signed comparisons for pointers (as long as objects can't
cross the -1..0 boundary).
Post by Kaz Kylheku
Anyway, even someone who doesn't care about maximal portability cannot
use this.
Yep.
--
Keith Thompson (The_Other_Keith) Keith.S.Thompson+***@gmail.com
Working, but not speaking, for Philips Healthcare
void Void(void) { Void(); } /* The recursive call of the void */
Kaz Kylheku
2020-09-11 15:43:50 UTC
Permalink
Post by Keith Thompson
Post by Kaz Kylheku
Post by Keith Thompson
[...]
Post by Scott Lurndal
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
mmap() returns a void*. Pointer values are neither negative nor
positive. A pointer value might have a representation equivalent to
that of a negative integer, but if you write
if (mmap(...) < 0) // replace ... by valid arguments
your code won't compile.
With GCC, a popular compiler, I had to use -pedantic to obtain
mmap.c:8:11: warning: ordered comparison of pointer with integer zero [-Wpedantic]
if (mem < 0)
^
Right. Rather than "won't compile" I should have written something like
"will be diagnosed by a conforming compiler". (gcc in its
non-conforming default mode permits a lot of things that are constraint
violations. It hadn't occurred to me that that would be one of them.)
I've also not tried this before. I mean, I've written code that compares
pointers to different objects for inequality knowing that it's not
maximally portable (because it's so much better than the portable
alternative). But never tried with a null pointer constant like this.

However, I suspected it would be accepted because GCC does support
inequality comparisons on pointers regardless of whether they point to
the same array-like object, and this is just a special case of that.

It would be useful to issue a diagnostic that the comparison
is always true due to the range of the type. You know the one.
--
TXR Programming Lanuage: http://nongnu.org/txr
Music DIY Mailing List: http://www.kylheku.com/diy
ADA MP-1 Mailing List: http://www.kylheku.com/mp1
James Kuyper
2020-09-11 20:20:10 UTC
Permalink
...
Post by Kaz Kylheku
Post by Keith Thompson
Post by Kaz Kylheku
Post by Keith Thompson
mmap() returns a void*. Pointer values are neither negative nor
positive. A pointer value might have a representation equivalent to
that of a negative integer, but if you write
if (mmap(...) < 0) // replace ... by valid arguments
your code won't compile.
With GCC, a popular compiler, I had to use -pedantic to obtain
mmap.c:8:11: warning: ordered comparison of pointer with integer zero [-Wpedantic]
if (mem < 0)
^
Right. Rather than "won't compile" I should have written something like
"will be diagnosed by a conforming compiler". (gcc in its
non-conforming default mode permits a lot of things that are constraint
violations. It hadn't occurred to me that that would be one of them.)
I've also not tried this before. I mean, I've written code that compares
pointers to different objects for inequality knowing that it's not
maximally portable (because it's so much better than the portable
alternative). But never tried with a null pointer constant like this.
There's a constraint that applies to comparisons for relative order:
"One of the following shall hold:
— both operands have real type; or
— both operands are pointers to qualified or unqualified versions of
compatible object types." (6.5.8p2).

In this case, the left operand has a pointer type, while the right
operand has a real type, so this is a constraint violation. In many
contexts (such as equality and inequality comparisons), there's special
wording for null pointer constants, which get implicitly get converted
into null pointers of the appropriate type - but no such wording applies
when comparing for relative order.

If you replaced 0 with a pointer expression such as (void*)0, there's
still another problem. Such comparisons only have defined behavior for
two pointers that both point into or one past the end of the same
object. (C standard 6.5.8p5). A null pointer never qualifies.
Kenny McCormack
2020-09-11 13:54:12 UTC
Permalink
In article <Z9s6H.457674$***@fx47.iad>,
Scott Lurndal <***@pacbell.net> wrote:
...
Post by Scott Lurndal
No, system calls return -1 on error. There are no other cases where
a system call returns a negative value on error. In fact, some system
calls return a negative value on success (e.g. mmap() when the mapped
address is in the top half of the virtual address space).
We've gotten confused between invocations of system(3) and "system calls".

Until this point in this thread, nobody was talking about (low-level)
system calls. But, of course, from here on in, people had no problem
yammering about off-topic nonsense.
--
A pervert, a racist, and a con man walk into a bar...

Bartender says, "What will you have, Donald!"
Siri Cruise
2020-09-11 15:19:55 UTC
Permalink
Post by Kenny McCormack
A pervert, a racist, and a con man walk into a bar...
Bartender says, "What will you have, Donald!"
That's a blatant lie. A real bartender will demand payment in
advance before taking an order from Donald.
--
:-<> Siri Seal of Disavowal #000-001. Disavowed. Denied. Deleted. @
'I desire mercy, not sacrifice.' /|\
The first law of discordiamism: The more energy This post / \
to make order is nore energy made into entropy. insults Islam. Mohammed
Kenny McCormack
2020-09-11 20:10:57 UTC
Permalink
Post by Siri Cruise
Post by Kenny McCormack
A pervert, a racist, and a con man walk into a bar...
Bartender says, "What will you have, Donald!"
That's a blatant lie. A real bartender will demand payment in
advance before taking an order from Donald.
That's for sure. Good point.
--
(Cruz certainly has an odd face) ... it looks like someone sewed pieces of a
waterlogged Reagan mask together at gunpoint ...

http://www.rollingstone.com/politics/news/how-america-made-donald-trump-unstoppable-20160224
Rainer Weikusat
2020-09-10 15:35:46 UTC
Permalink
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
I then want to be check the integer value returned by the program "grep" to see if it succeeded.
[...]
Post by Frederick Gotham
int const status = system("grep sdcard /proc/mounts");
if ( (-1 != status) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
There are really five different cases here:

If the return value is -1, the system call (the library function system,
not "a system call") failed before a program was executed. (a)

If an exit status of 127 is returned, either the shell could not be
executed or the shell could not find the program it was supposed to
execute. (b)

An exit status of 1 means grep ran successfully but didn't find anything
matching the pattern. (c)

An exit status of 2 means grep failed due to some error. (d)

An exit status of 0 means grep ran successfully and the pattern
matched. (e)

a, b and c could be grouped together as "failed to run grep". One might
want to treat this differently than c (grep ran but there were no
matches).

A considerably amount of variability/ uncertainty can be eliminated here by not using
system.

Stilistic remark:

if (-1 == status)

means "I'm really stuck in the 1970s together with Nikolaus Wirth and
refuse (!!1) to accept that := never became as popular as I think it
deserved !!2".

IMHO, pointless political statements of this kind should be avoided in
code, especially, if they result in convoluted semantics.
Scott Lurndal
2020-09-10 16:04:41 UTC
Permalink
Post by Frederick Gotham
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that instead of checking for -1. This seems to be quite common on sites like Stack Overflow.
Because they're lazy.

If an interface is documented to return -1 when an error is detected, that's the value that
should be checked for.

Interfaces such as 'mmap(2)' and 'lseek(2)' can return successful values that will
fail (diag < 0) type checks.
Keith Thompson
2020-09-10 20:36:35 UTC
Permalink
Post by Scott Lurndal
Post by Frederick Gotham
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that
instead of checking for -1. This seems to be quite common on sites
like Stack Overflow.
Because they're lazy.
If an interface is documented to return -1 when an error is detected,
that's the value that should be checked for.
Interfaces such as 'mmap(2)' and 'lseek(2)' can return successful values that will
fail (diag < 0) type checks.
(mmap(...) < 0) won't compile, since mmap() returns a void*.

For lseek:

Upon successful completion, the resulting offset, as measured in
bytes from the beginning of the file, shall be returned. Otherwise,
-1 shall be returned, errno shall be set to indicate the error, and
the file offset shall remain unchanged.

How would a successful result be less than zero?
--
Keith Thompson (The_Other_Keith) Keith.S.Thompson+***@gmail.com
Working, but not speaking, for Philips Healthcare
void Void(void) { Void(); } /* The recursive call of the void */
Scott Lurndal
2020-09-10 23:19:59 UTC
Permalink
Post by Keith Thompson
Post by Scott Lurndal
Post by Frederick Gotham
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that
instead of checking for -1. This seems to be quite common on sites
like Stack Overflow.
Because they're lazy.
If an interface is documented to return -1 when an error is detected,
that's the value that should be checked for.
Interfaces such as 'mmap(2)' and 'lseek(2)' can return successful values that will
fail (diag < 0) type checks.
(mmap(...) < 0) won't compile, since mmap() returns a void*.
People do cast it, I've seen it; for whatever reason they don't read
the documentation, or don't understand MAP_FAILED.
Post by Keith Thompson
Upon successful completion, the resulting offset, as measured in
bytes from the beginning of the file, shall be returned. Otherwise,
-1 shall be returned, errno shall be set to indicate the error, and
the file offset shall remain unchanged.
How would a successful result be less than zero?
When it is more than 2GB on a 32-bit system. The first time I
encountered it was when opening /dev/mem and seeking to offsets
greater than 2GB (before pread/pwrite showedup on 1003.4).
Keith Thompson
2020-09-11 00:46:23 UTC
Permalink
[...]
Post by Scott Lurndal
Post by Keith Thompson
Upon successful completion, the resulting offset, as measured in
bytes from the beginning of the file, shall be returned. Otherwise,
-1 shall be returned, errno shall be set to indicate the error, and
the file offset shall remain unchanged.
How would a successful result be less than zero?
When it is more than 2GB on a 32-bit system. The first time I
encountered it was when opening /dev/mem and seeking to offsets
greater than 2GB (before pread/pwrite showedup on 1003.4).
POSIX says that lseek() shall fail and set errno to EOVERFLOW if "The
resulting file offset would be a value which cannot be represented
correctly in an object of type off_t.". I wouldn't be too surprised if
some implementations were non-conforming.
--
Keith Thompson (The_Other_Keith) Keith.S.Thompson+***@gmail.com
Working, but not speaking, for Philips Healthcare
void Void(void) { Void(); } /* The recursive call of the void */
Scott Lurndal
2020-09-11 23:06:56 UTC
Permalink
Post by Keith Thompson
[...]
Post by Scott Lurndal
Post by Keith Thompson
Upon successful completion, the resulting offset, as measured in
bytes from the beginning of the file, shall be returned. Otherwise,
-1 shall be returned, errno shall be set to indicate the error, and
the file offset shall remain unchanged.
How would a successful result be less than zero?
When it is more than 2GB on a 32-bit system. The first time I
encountered it was when opening /dev/mem and seeking to offsets
greater than 2GB (before pread/pwrite showedup on 1003.4).
POSIX says that lseek() shall fail and set errno to EOVERFLOW if "The
resulting file offset would be a value which cannot be represented
correctly in an object of type off_t.". I wouldn't be too surprised if
some implementations were non-conforming.
This was about the time of the LFS (Large File Summit - I attended as a company rep),
from which I believe the EOVERFLOW error derived - I don't have my SVID handy to check.

Any access to files larger than 2GB was pretty much implementation dependent
prior to LFS.

Geoff Clare
2020-09-11 12:34:48 UTC
Permalink
Post by Keith Thompson
Upon successful completion, the resulting offset, as measured in
bytes from the beginning of the file, shall be returned. Otherwise,
-1 shall be returned, errno shall be set to indicate the error, and
the file offset shall remain unchanged.
How would a successful result be less than zero?
Dig deeper:

ERRORS

The lseek() function shall fail if:

[EINVAL]
The whence argument is not a proper value, or the resulting file
offset would be negative for a regular file, block special file,
or directory.

RATIONALE

An invalid file offset that would cause [EINVAL] to be returned
may be both implementation-defined and device-dependent (for
example, memory may have few invalid values). A negative file
offset may be valid for some devices in some implementations.

The POSIX.1-1990 standard did not specifically prohibit lseek()
from returning a negative offset. Therefore, an application was
required to clear errno prior to the call and check errno upon
return to determine whether a return value of (off_t)-1 is a
negative offset or an indication of an error condition. The
standard developers did not wish to require this action on the
part of a conforming application, and chose to require that errno
be set to [EINVAL] when the resulting file offset would be
negative for a regular file, block special file, or directory.

So a successful return of a negative offset is permitted for a
character "special file" (device).
--
Geoff Clare <***@gclare.org.uk>
Kaz Kylheku
2020-09-10 19:50:23 UTC
Permalink
Post by Frederick Gotham
if ( (status < 0) && WIFEXITED(status) && EXIT_SUCCESS == WEXITSTATUS(status) )
{
puts("grep succeeded");
}
I don't see why they're checking for "less than zero" like that instead of checking for -1. This seems to be quite common on sites like Stack Overflow.
That looks like buggy nonsense. In the expected case when the command exits
normally with a successful termination status, system returns precisely zero,
which is not less than zero.

I can explore some of the usual behaviors quickly via FFI in TXR Lisp though
not the -1 case whereby fork fails:

This is the TXR Lisp interactive listener of TXR 243.
Quit with :quit or Ctrl-D on an empty line. Ctrl-X ? for cheatsheet.
1> (with-dyn-lib nil
(deffi system "system" int (str)))
#:lib-0009

Basic test:

2> (system "echo working")
working
0

Try the true builtin:

3> (system "true")
0

False:

4> (system "false")
256

Nonexistent program:

5> (system "/usr/bin/nonexistent")
sh: 1: /usr/bin/nonexistent: not found
32512

Bad shell syntax:

6> (system "(badshellsyntax ")
sh: 1: Syntax error: end of file unexpected (expecting ")")
512

In the nonexistent program case, the termination status is 127:

7> (w-ifexited 32512)
t
8> (w-ifsignaled 32512)
nil
9> (w-exitstatus 32512)
127

What status is the 512 status value from bad syntax:

10> (w-exitstatus 512)
2

Check that system(NULL) returns nonzero to indicate that the interpreter is
available:

11> (system nil)
1

As you can see, the failed exit statuses are positive.

That might not be the case for all possible failed exit statuses. However, on
Linux, the status word is just 16 bits. Values of the termination status above
255 are truncated:

12> (system "exit 255")
65280
13> (system "exit 65535")
65280

I don't think that any manner of termination uses any higher bits than 16 on
Linux, so there are only positive values.

Still, I would check for the exact value of -1, as you are doing, and not
assume that any negative value means that creating the child process failed.
Loading...