svn commit: r356725 - head/usr.bin/diff

[Ian Lepore]

On Tue, 2020-01-14 at 08:22 +0000, Baptiste Daroussin wrote:
> Author: bapt
> Date: Tue Jan 14 08:22:28 2020
> New Revision: 356725
> URL: https://svnweb.freebsd.org/changeset/base/356725
> 
> Log:
>   When system calls indicate an error they return -1, not some
> arbitrary
>   value < 0.  errno is only updated in this case.
>   

What's the point of these changes, other than almost certainly leading
to worse code generation?

In most instruction sets, you can test for a value < 0 without using
any instructions, you only need to examine the condition flags after
loading the value.  To compare equal to -1 typically requires at least
1 extra instruction, and on risc architectures typically at least two
extra (load -1 to a register then compare).

-- Ian


>   Obtained from:	OpenBSD
>   MFC after:	3 days
> 
> Modified:
>   head/usr.bin/diff/diff.c
>   head/usr.bin/diff/diffreg.c
> 
> Modified: head/usr.bin/diff/diff.c
> =====================================================================
> =========
> --- head/usr.bin/diff/diff.c	Tue Jan 14 08:18:04 2020	(r356724)
> +++ head/usr.bin/diff/diff.c	Tue Jan 14 08:22:28 2020	(r356725)
> @@ -1,4 +1,4 @@
> -/*	$OpenBSD: diff.c,v 1.65 2015/12/29 19:04:46 gsoares Exp $	*/
> +/*	$OpenBSD: diff.c,v 1.67 2019/06/28 13:35:00 deraadt Exp $	*/
>  
>  /*
>   * Copyright (c) 2003 Todd C. Miller <Todd.Miller at courtesan.com>
> @@ -316,12 +316,12 @@ main(int argc, char **argv)
>  	} else {
>  		if (S_ISDIR(stb1.st_mode)) {
>  			argv[0] = splice(argv[0], argv[1]);
> -			if (stat(argv[0], &stb1) < 0)
> +			if (stat(argv[0], &stb1) == -1)
>  				err(2, "%s", argv[0]);
>  		}
>  		if (S_ISDIR(stb2.st_mode)) {
>  			argv[1] = splice(argv[1], argv[0]);
> -			if (stat(argv[1], &stb2) < 0)
> +			if (stat(argv[1], &stb2) == -1)
>  				err(2, "%s", argv[1]);
>  		}
>  		print_status(diffreg(argv[0], argv[1], dflags, 1),
> argv[0],
> 
> Modified: head/usr.bin/diff/diffreg.c
> =====================================================================
> =========
> --- head/usr.bin/diff/diffreg.c	Tue Jan 14 08:18:04 2020	(r356
> 724)
> +++ head/usr.bin/diff/diffreg.c	Tue Jan 14 08:22:28 2020	(r356
> 725)
> @@ -1,4 +1,4 @@
> -/*	$OpenBSD: diffreg.c,v 1.92 2019/06/28 05:35:34 deraadt Exp $	
> */
> +/*	$OpenBSD: diffreg.c,v 1.93 2019/06/28 13:35:00 deraadt Exp $	
> */
>  
>  /*-
>   * SPDX-License-Identifier: BSD-4-Clause
> @@ -277,7 +277,7 @@ diffreg(char *file1, char *file2, int flags, int
> capsi
>  	else {
>  		if (!S_ISREG(stb1.st_mode)) {
>  			if ((f1 = opentemp(file1)) == NULL ||
> -			    fstat(fileno(f1), &stb1) < 0) {
> +			    fstat(fileno(f1), &stb1) == -1) {
>  				warn("%s", file1);
>  				status |= 2;
>  				goto closem;
> @@ -298,7 +298,7 @@ diffreg(char *file1, char *file2, int flags, int
> capsi
>  	else {
>  		if (!S_ISREG(stb2.st_mode)) {
>  			if ((f2 = opentemp(file2)) == NULL ||
> -			    fstat(fileno(f2), &stb2) < 0) {
> +			    fstat(fileno(f2), &stb2) == -1) {
>  				warn("%s", file2);
>  				status |= 2;
>  				goto closem;
> @@ -446,7 +446,7 @@ opentemp(const char *f)
>  
>  	if (strcmp(f, "-") == 0)
>  		ifd = STDIN_FILENO;
> -	else if ((ifd = open(f, O_RDONLY, 0644)) < 0)
> +	else if ((ifd = open(f, O_RDONLY, 0644)) == -1)
>  		return (NULL);
>  
>  	(void)strlcpy(tempfile, _PATH_TMP "/diff.XXXXXXXX",
> sizeof(tempfile));
> @@ -942,7 +942,7 @@ preadline(int fd, size_t rlen, off_t off)
>  	ssize_t nr;
>  
>  	line = xmalloc(rlen + 1);
> -	if ((nr = pread(fd, line, rlen, off)) < 0)
> +	if ((nr = pread(fd, line, rlen, off)) == -1)
>  		err(2, "preadline");
>  	if (nr > 0 && line[nr-1] == '\n')
>  		nr--;