[kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Felix Janda]

---
 src/kbdinfo.c        | 26 ++++++++++++++++----------
 src/setvtrgb.c       | 42 ++++++++++++++++++++++++------------------
 src/vlock/auth.c     |  2 --
 src/vlock/pam.c      |  6 ++----
 src/vlock/username.c | 12 ++++++++----
 src/vlock/vlock.c    | 11 ++++++-----
 src/vlock/vt.c       |  5 ++---
 7 files changed, 58 insertions(+), 46 deletions(-)

diff --git a/src/kbdinfo.c b/src/kbdinfo.c
index e5f122d..dfce591 100644
--- a/src/kbdinfo.c
+++ b/src/kbdinfo.c
@@ -1,6 +1,4 @@
 #include <stdio.h>
-#include <errno.h>
-#include <error.h>
 #include <unistd.h>
 #include <sys/ioctl.h>
 #include <linux/kd.h>
@@ -74,8 +72,10 @@ main(int argc, char **argv) {
 	fd = getfd(console);
 
 	if (!strcasecmp("GETMODE", action)) {
-		if (ioctl(fd, KDGETMODE, &mode) == -1)
-			error(EXIT_FAILURE, errno, "ioctl");
+		if (ioctl(fd, KDGETMODE, &mode) == -1) {
+			perror("ioctl");
+			exit(EXIT_FAILURE);
+		}
 
 		switch (mode) {
 			case KD_TEXT:		rc = answer("text");		break;
@@ -83,8 +83,10 @@ main(int argc, char **argv) {
 		}
 
 	} else if (!strcasecmp("GKBMODE", action)) {
-		if (ioctl(fd, KDGKBMODE, &mode) == -1)
-			error(EXIT_FAILURE, errno, "ioctl");
+		if (ioctl(fd, KDGKBMODE, &mode) == -1) {
+			perror("ioctl");
+			exit(EXIT_FAILURE);
+		}
 
 		switch (mode) {
 			case K_RAW:		rc = answer("raw");		break;
@@ -94,8 +96,10 @@ main(int argc, char **argv) {
 		}
 
 	} else if (!strcasecmp("GKBMETA", action)) {
-		if (ioctl(fd, KDGKBMETA, &mode) == -1)
-			error(EXIT_FAILURE, errno, "ioctl");
+		if (ioctl(fd, KDGKBMETA, &mode) == -1) {
+			perror("ioctl");
+			exit(EXIT_FAILURE);
+		}
 
 		switch (mode) {
 			case K_METABIT:		rc = answer("metabit");		break;
@@ -103,8 +107,10 @@ main(int argc, char **argv) {
 		}
 
 	} else if (!strcasecmp("GKBLED", action)) {
-		if (ioctl(fd, KDGKBLED, &flags) == -1)
-			error(EXIT_FAILURE, errno, "ioctl");
+		if (ioctl(fd, KDGKBLED, &flags) == -1) {
+			perror("ioctl");
+			exit(EXIT_FAILURE);
+		}
 
 		mode = (flags & 0x7);
 
diff --git a/src/setvtrgb.c b/src/setvtrgb.c
index 3fa03a7..e369702 100644
--- a/src/setvtrgb.c
+++ b/src/setvtrgb.c
@@ -4,8 +4,6 @@
 #include <getopt.h>
 #include <sys/ioctl.h>
 #include <linux/kd.h>
-#include <errno.h>
-#include <error.h>
 #include "kbd.h"
 #include "getfd.h"
 #include "nls.h"
@@ -63,26 +61,30 @@ parse_file(FILE *fd, const char *filename)
 		for (cols = 0; cols < 16; cols++) {
 			if ((c = fscanf(fd, "%u", &val)) != 1) {
 				if (c == EOF)
-					error(EXIT_FAILURE, errno, "fscanf");
-
-				error(EXIT_FAILURE, 0, _("Error: %s: Invalid value in field %u in line %u."),
-				      filename, rows + 1, cols + 1);
+					perror("fscanf");
+				else
+					fprintf(stderr, _("Error: %s: Invalid value in field %u in line %u."),
+					      filename, rows + 1, cols + 1);
+				exit(EXIT_FAILURE);
 			}
 
 			cmap[rows + cols * 3] = (unsigned char) val;
 
-			if (cols < 15 && fgetc(fd) != ',')
-				error(EXIT_FAILURE, 0, _("Error: %s: Insufficient number of fields in line %u."),
+			if (cols < 15 && fgetc(fd) != ',') {
+				fprintf(stderr, _("Error: %s: Insufficient number of fields in line %u."),
 				      filename, rows + 1);
+				exit(EXIT_FAILURE);
+			}
 		}
 
-		if ((c = fgetc(fd)) == EOF)
-			error(EXIT_FAILURE, 0, _("Error: %s: Line %u has ended unexpectedly.\n"),
-			      filename, rows + 1); 
+		if ((c = fgetc(fd)) == EOF) {
+			fprintf(stderr, _("Error: %s: Line %u has ended unexpectedly.\n"), filename, rows + 1);
+		} else if (c != '\n') {
+			fprintf(stderr, _("Error: %s: Line %u is too long.\n"), filename, rows + 1);
+		} else
+			continue;
 
-		if (c != '\n')
-			error(EXIT_FAILURE, 0, _("Error: %s: Line %u is too long.\n"),
-			      filename, rows + 1);
+		exit(EXIT_FAILURE);
 	}
 }
 
@@ -122,8 +124,10 @@ main(int argc, char **argv) {
 		parse_file(stdin, "stdin");
 
 	} else {
-		if ((f = fopen(file, "r")) == NULL)
-			error(EXIT_FAILURE, errno, "fopen");
+		if ((f = fopen(file, "r")) == NULL) {
+			perror("fopen");
+			return EXIT_FAILURE;
+		}
 
 		parse_file(f, file);
 		fclose(f);
@@ -132,8 +136,10 @@ main(int argc, char **argv) {
 	fd = getfd(NULL);
 
 	/* Apply the color map to the tty via ioctl */
-	if (ioctl(fd, PIO_CMAP, colormap) == -1)
-		error(EXIT_FAILURE, errno, "ioctl");
+	if (ioctl(fd, PIO_CMAP, colormap) == -1) {
+		perror("ioctl");
+		return EXIT_FAILURE;
+	}
 
 	close(fd);
 
diff --git a/src/vlock/auth.c b/src/vlock/auth.c
index eddce51..4964b30 100644
--- a/src/vlock/auth.c
+++ b/src/vlock/auth.c
@@ -21,8 +21,6 @@
   Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
 */
 
-#include <errno.h>
-#include <error.h>
 #include <stdio.h>
 #include <string.h>
 #include <stdlib.h>
diff --git a/src/vlock/pam.c b/src/vlock/pam.c
index ab255db..a178e94 100644
--- a/src/vlock/pam.c
+++ b/src/vlock/pam.c
@@ -21,8 +21,6 @@
 */
 
 #include <stdio.h>
-#include <errno.h>
-#include <error.h>
 #include <stdlib.h>
 #include <syslog.h>
 
@@ -48,7 +46,7 @@ init_pam (const char *username, const char *tty, int log)
 		if (log)
 			syslog (LOG_WARNING, "pam_start failed: %m");
 		else
-			error (EXIT_SUCCESS, errno, "pam_start");
+			perror ("pam_start");
 		return 0;
 	}
 
@@ -59,7 +57,7 @@ init_pam (const char *username, const char *tty, int log)
 			syslog (LOG_WARNING, "pam_set_item: %s",
 				pam_strerror (pamh, rc));
 		else
-			error (EXIT_SUCCESS, 0, "pam_set_item: %s",
+			fprintf (stderr, "pam_set_item: %s",
 			       pam_strerror (pamh, rc));
 		pam_end (pamh, rc);
 		return 0;
diff --git a/src/vlock/username.c b/src/vlock/username.c
index 9626946..6f3c712 100644
--- a/src/vlock/username.c
+++ b/src/vlock/username.c
@@ -21,8 +21,6 @@
 */
 
 #include <stdio.h>
-#include <errno.h>
-#include <error.h>
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -54,11 +52,17 @@ get_username (void)
 		pw = getpwuid (uid);
 
 	if (!pw)
-		error (EXIT_FAILURE, 0, _("unrecognized user"));
+	{
+		fprintf (stderr, _("unrecognized user"));
+		return NULL;
+	}
 
 	name = strdup (pw->pw_name);
 	if (!name)
-		error (EXIT_FAILURE, errno, "strdup");
+	{
+		perror ("strdup");
+		return NULL;
+	}
 
 	endpwent ();
 	return name;
diff --git a/src/vlock/vlock.c b/src/vlock/vlock.c
index c2c4158..559dd0c 100644
--- a/src/vlock/vlock.c
+++ b/src/vlock/vlock.c
@@ -22,8 +22,6 @@
 */
 
 #include <stdio.h>
-#include <errno.h>
-#include <error.h>
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -51,16 +49,19 @@ main (int ac, char *const av[])
 
 	/* 2nd, find out tty name... */
 	tty = ttyname (STDIN_FILENO);
-	if (!tty)
+	if (!tty) {
 		/* stdin is not a tty, so no need to try. */
-		error (EXIT_FAILURE, 0, _("stdin is not a tty"));
+		fprintf (stderr,  _("stdin is not a tty"));
+		exit (EXIT_FAILURE);
+	}
 
 	/* ... and strip its /dev/ prefix. */
 	if (!strncmp (tty, dev_prefix, sizeof (dev_prefix) - 1))
 		tty += sizeof (dev_prefix) - 1;
 
 	/* 3rd, get username for PAM. */
-	username = get_username ();
+	if (!(username = get_username ()))
+		exit (EXIT_FAILURE);
 
 	/* 4th, initialize system logger. */
 	openlog ("vlock", LOG_PID, LOG_AUTH);
diff --git a/src/vlock/vt.c b/src/vlock/vt.c
index 4e5282b..96ec9a1 100644
--- a/src/vlock/vt.c
+++ b/src/vlock/vt.c
@@ -23,7 +23,6 @@
 
 #include <stdio.h>
 #include <errno.h>
-#include <error.h>
 #include <string.h>
 #include <stdlib.h>
 #include <unistd.h>
@@ -136,7 +135,7 @@ init_vt (const char *tty)
 	vfd = open (dev_tty, O_RDWR);
 	if (vfd < 0)
 	{
-		error (EXIT_SUCCESS, errno, "could not open %s", dev_tty);
+		fprintf (stderr, "could not open %s: %s", dev_tty, strerror(errno));
 		return 0;
 	}
 
@@ -187,7 +186,7 @@ init_vt (const char *tty)
 		/* Set mode of active vt. */
 		if (ioctl (vfd, VT_SETMODE, &vtm) < 0)
 		{
-			error (EXIT_SUCCESS, errno, "ioctl VT_SETMODE");
+			perror ("ioctl VT_SETMODE");
 			return 0;
 		}
 	}
-- 
2.0.5

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

22.04.2015 22:06, Felix Janda пишет:
> ---
>  src/kbdinfo.c        | 26 ++++++++++++++++----------
>  src/setvtrgb.c       | 42 ++++++++++++++++++++++++------------------
>  src/vlock/auth.c     |  2 --
>  src/vlock/pam.c      |  6 ++----
>  src/vlock/username.c | 12 ++++++++----
>  src/vlock/vlock.c    | 11 ++++++-----
>  src/vlock/vt.c       |  5 ++---
>  7 files changed, 58 insertions(+), 46 deletions(-)

This patch does not an equivalent substitute. I think it's better to
implement a local version of error().

> diff --git a/src/kbdinfo.c b/src/kbdinfo.c
> index e5f122d..dfce591 100644
> --- a/src/kbdinfo.c
> +++ b/src/kbdinfo.c
> @@ -1,6 +1,4 @@
>  #include <stdio.h>
> -#include <errno.h>
> -#include <error.h>
>  #include <unistd.h>
>  #include <sys/ioctl.h>
>  #include <linux/kd.h>
> @@ -74,8 +72,10 @@ main(int argc, char **argv) {
>  	fd = getfd(console);
>  
>  	if (!strcasecmp("GETMODE", action)) {
> -		if (ioctl(fd, KDGETMODE, &mode) == -1)
> -			error(EXIT_FAILURE, errno, "ioctl");
> +		if (ioctl(fd, KDGETMODE, &mode) == -1) {
> +			perror("ioctl");
> +			exit(EXIT_FAILURE);
> +		}
>  
>  		switch (mode) {
>  			case KD_TEXT:		rc = answer("text");		break;
> @@ -83,8 +83,10 @@ main(int argc, char **argv) {
>  		}
>  
>  	} else if (!strcasecmp("GKBMODE", action)) {
> -		if (ioctl(fd, KDGKBMODE, &mode) == -1)
> -			error(EXIT_FAILURE, errno, "ioctl");
> +		if (ioctl(fd, KDGKBMODE, &mode) == -1) {
> +			perror("ioctl");
> +			exit(EXIT_FAILURE);
> +		}
>  
>  		switch (mode) {
>  			case K_RAW:		rc = answer("raw");		break;
> @@ -94,8 +96,10 @@ main(int argc, char **argv) {
>  		}
>  
>  	} else if (!strcasecmp("GKBMETA", action)) {
> -		if (ioctl(fd, KDGKBMETA, &mode) == -1)
> -			error(EXIT_FAILURE, errno, "ioctl");
> +		if (ioctl(fd, KDGKBMETA, &mode) == -1) {
> +			perror("ioctl");
> +			exit(EXIT_FAILURE);
> +		}
>  
>  		switch (mode) {
>  			case K_METABIT:		rc = answer("metabit");		break;
> @@ -103,8 +107,10 @@ main(int argc, char **argv) {
>  		}
>  
>  	} else if (!strcasecmp("GKBLED", action)) {
> -		if (ioctl(fd, KDGKBLED, &flags) == -1)
> -			error(EXIT_FAILURE, errno, "ioctl");
> +		if (ioctl(fd, KDGKBLED, &flags) == -1) {
> +			perror("ioctl");
> +			exit(EXIT_FAILURE);
> +		}
>  
>  		mode = (flags & 0x7);
>  
> diff --git a/src/setvtrgb.c b/src/setvtrgb.c
> index 3fa03a7..e369702 100644
> --- a/src/setvtrgb.c
> +++ b/src/setvtrgb.c
> @@ -4,8 +4,6 @@
>  #include <getopt.h>
>  #include <sys/ioctl.h>
>  #include <linux/kd.h>
> -#include <errno.h>
> -#include <error.h>
>  #include "kbd.h"
>  #include "getfd.h"
>  #include "nls.h"
> @@ -63,26 +61,30 @@ parse_file(FILE *fd, const char *filename)
>  		for (cols = 0; cols < 16; cols++) {
>  			if ((c = fscanf(fd, "%u", &val)) != 1) {
>  				if (c == EOF)
> -					error(EXIT_FAILURE, errno, "fscanf");
> -
> -				error(EXIT_FAILURE, 0, _("Error: %s: Invalid value in field %u in line %u."),
> -				      filename, rows + 1, cols + 1);
> +					perror("fscanf");
> +				else
> +					fprintf(stderr, _("Error: %s: Invalid value in field %u in line %u."),
> +					      filename, rows + 1, cols + 1);
> +				exit(EXIT_FAILURE);
>  			}
>  
>  			cmap[rows + cols * 3] = (unsigned char) val;
>  
> -			if (cols < 15 && fgetc(fd) != ',')
> -				error(EXIT_FAILURE, 0, _("Error: %s: Insufficient number of fields in line %u."),
> +			if (cols < 15 && fgetc(fd) != ',') {
> +				fprintf(stderr, _("Error: %s: Insufficient number of fields in line %u."),
>  				      filename, rows + 1);
> +				exit(EXIT_FAILURE);
> +			}
>  		}
>  
> -		if ((c = fgetc(fd)) == EOF)
> -			error(EXIT_FAILURE, 0, _("Error: %s: Line %u has ended unexpectedly.\n"),
> -			      filename, rows + 1); 
> +		if ((c = fgetc(fd)) == EOF) {
> +			fprintf(stderr, _("Error: %s: Line %u has ended unexpectedly.\n"), filename, rows + 1);
> +		} else if (c != '\n') {
> +			fprintf(stderr, _("Error: %s: Line %u is too long.\n"), filename, rows + 1);
> +		} else
> +			continue;
>  
> -		if (c != '\n')
> -			error(EXIT_FAILURE, 0, _("Error: %s: Line %u is too long.\n"),
> -			      filename, rows + 1);
> +		exit(EXIT_FAILURE);
>  	}
>  }
>  
> @@ -122,8 +124,10 @@ main(int argc, char **argv) {
>  		parse_file(stdin, "stdin");
>  
>  	} else {
> -		if ((f = fopen(file, "r")) == NULL)
> -			error(EXIT_FAILURE, errno, "fopen");
> +		if ((f = fopen(file, "r")) == NULL) {
> +			perror("fopen");
> +			return EXIT_FAILURE;
> +		}
>  
>  		parse_file(f, file);
>  		fclose(f);
> @@ -132,8 +136,10 @@ main(int argc, char **argv) {
>  	fd = getfd(NULL);
>  
>  	/* Apply the color map to the tty via ioctl */
> -	if (ioctl(fd, PIO_CMAP, colormap) == -1)
> -		error(EXIT_FAILURE, errno, "ioctl");
> +	if (ioctl(fd, PIO_CMAP, colormap) == -1) {
> +		perror("ioctl");
> +		return EXIT_FAILURE;
> +	}
>  
>  	close(fd);
>  
> diff --git a/src/vlock/auth.c b/src/vlock/auth.c
> index eddce51..4964b30 100644
> --- a/src/vlock/auth.c
> +++ b/src/vlock/auth.c
> @@ -21,8 +21,6 @@
>    Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
>  */
>  
> -#include <errno.h>
> -#include <error.h>
>  #include <stdio.h>
>  #include <string.h>
>  #include <stdlib.h>
> diff --git a/src/vlock/pam.c b/src/vlock/pam.c
> index ab255db..a178e94 100644
> --- a/src/vlock/pam.c
> +++ b/src/vlock/pam.c
> @@ -21,8 +21,6 @@
>  */
>  
>  #include <stdio.h>
> -#include <errno.h>
> -#include <error.h>
>  #include <stdlib.h>
>  #include <syslog.h>
>  
> @@ -48,7 +46,7 @@ init_pam (const char *username, const char *tty, int log)
>  		if (log)
>  			syslog (LOG_WARNING, "pam_start failed: %m");
>  		else
> -			error (EXIT_SUCCESS, errno, "pam_start");
> +			perror ("pam_start");
>  		return 0;
>  	}
>  
> @@ -59,7 +57,7 @@ init_pam (const char *username, const char *tty, int log)
>  			syslog (LOG_WARNING, "pam_set_item: %s",
>  				pam_strerror (pamh, rc));
>  		else
> -			error (EXIT_SUCCESS, 0, "pam_set_item: %s",
> +			fprintf (stderr, "pam_set_item: %s",
>  			       pam_strerror (pamh, rc));
>  		pam_end (pamh, rc);
>  		return 0;
> diff --git a/src/vlock/username.c b/src/vlock/username.c
> index 9626946..6f3c712 100644
> --- a/src/vlock/username.c
> +++ b/src/vlock/username.c
> @@ -21,8 +21,6 @@
>  */
>  
>  #include <stdio.h>
> -#include <errno.h>
> -#include <error.h>
>  #include <string.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> @@ -54,11 +52,17 @@ get_username (void)
>  		pw = getpwuid (uid);
>  
>  	if (!pw)
> -		error (EXIT_FAILURE, 0, _("unrecognized user"));
> +	{
> +		fprintf (stderr, _("unrecognized user"));
> +		return NULL;
> +	}
>  
>  	name = strdup (pw->pw_name);
>  	if (!name)
> -		error (EXIT_FAILURE, errno, "strdup");
> +	{
> +		perror ("strdup");
> +		return NULL;
> +	}
>  
>  	endpwent ();
>  	return name;
> diff --git a/src/vlock/vlock.c b/src/vlock/vlock.c
> index c2c4158..559dd0c 100644
> --- a/src/vlock/vlock.c
> +++ b/src/vlock/vlock.c
> @@ -22,8 +22,6 @@
>  */
>  
>  #include <stdio.h>
> -#include <errno.h>
> -#include <error.h>
>  #include <string.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> @@ -51,16 +49,19 @@ main (int ac, char *const av[])
>  
>  	/* 2nd, find out tty name... */
>  	tty = ttyname (STDIN_FILENO);
> -	if (!tty)
> +	if (!tty) {
>  		/* stdin is not a tty, so no need to try. */
> -		error (EXIT_FAILURE, 0, _("stdin is not a tty"));
> +		fprintf (stderr,  _("stdin is not a tty"));
> +		exit (EXIT_FAILURE);
> +	}
>  
>  	/* ... and strip its /dev/ prefix. */
>  	if (!strncmp (tty, dev_prefix, sizeof (dev_prefix) - 1))
>  		tty += sizeof (dev_prefix) - 1;
>  
>  	/* 3rd, get username for PAM. */
> -	username = get_username ();
> +	if (!(username = get_username ()))
> +		exit (EXIT_FAILURE);
>  
>  	/* 4th, initialize system logger. */
>  	openlog ("vlock", LOG_PID, LOG_AUTH);
> diff --git a/src/vlock/vt.c b/src/vlock/vt.c
> index 4e5282b..96ec9a1 100644
> --- a/src/vlock/vt.c
> +++ b/src/vlock/vt.c
> @@ -23,7 +23,6 @@
>  
>  #include <stdio.h>
>  #include <errno.h>
> -#include <error.h>
>  #include <string.h>
>  #include <stdlib.h>
>  #include <unistd.h>
> @@ -136,7 +135,7 @@ init_vt (const char *tty)
>  	vfd = open (dev_tty, O_RDWR);
>  	if (vfd < 0)
>  	{
> -		error (EXIT_SUCCESS, errno, "could not open %s", dev_tty);
> +		fprintf (stderr, "could not open %s: %s", dev_tty, strerror(errno));
>  		return 0;
>  	}
>  
> @@ -187,7 +186,7 @@ init_vt (const char *tty)
>  		/* Set mode of active vt. */
>  		if (ioctl (vfd, VT_SETMODE, &vtm) < 0)
>  		{
> -			error (EXIT_SUCCESS, errno, "ioctl VT_SETMODE");
> +			perror ("ioctl VT_SETMODE");
>  			return 0;
>  		}
>  	}
> 


-- 
Rgrds, legion

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

22.04.2015 22:06, Felix Janda пишет:
> ---
>  src/kbdinfo.c        | 26 ++++++++++++++++----------
>  src/setvtrgb.c       | 42 ++++++++++++++++++++++++------------------
>  src/vlock/auth.c     |  2 --
>  src/vlock/pam.c      |  6 ++----
>  src/vlock/username.c | 12 ++++++++----
>  src/vlock/vlock.c    | 11 ++++++-----
>  src/vlock/vt.c       |  5 ++---
>  7 files changed, 58 insertions(+), 46 deletions(-)

Applied, except for the last one. Please check out the master branch.

-- 
Rgrds, legion

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Felix Janda]

Alexey Gladkov wrote:
> 22.04.2015 22:06, Felix Janda пишет:
> > ---
> >  src/kbdinfo.c        | 26 ++++++++++++++++----------
> >  src/setvtrgb.c       | 42 ++++++++++++++++++++++++------------------
> >  src/vlock/auth.c     |  2 --
> >  src/vlock/pam.c      |  6 ++----
> >  src/vlock/username.c | 12 ++++++++----
> >  src/vlock/vlock.c    | 11 ++++++-----
> >  src/vlock/vt.c       |  5 ++---
> >  7 files changed, 58 insertions(+), 46 deletions(-)
> 
> Applied, except for the last one. Please check out the master branch.

Thanks for applying the other patches and for kbd_error()! Now everything
compiles fine with musl libc.

However kbd_error() does not behave correctly in the case that the first
argument is 0 (EXIT_SUCCESS). error() does _not_ terminate the program
if the first argument is EXIT_SUCCESS. Maybe have in addition a
kbd_warn() so that the __attribute__((noreturn)) can stay for kbd_error()
(then the exitnum argument would also become superfluous).

My excuse [1] for changing the error messages (no more program name) was
because the source is inconsistent. For example init_vt() in vlock/vt.c
or main() in kbdinfo.c mix fprintf(stderr) and error(). It seems to me
that error was mainly used as a convenient shorthand for
fprint(stderr) && exit(). (Having a shorthand makes sense.)

Thanks,
Felix

PS: Could you maybe note on the webpage that git.altlinux.org is the
main git repository and might be ahead of the git.kernel.org repo?

[1]: http://lists.altlinux.org/pipermail/kbd/2014-December/000487.html

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

29.04.2015 21:11, Felix Janda пишет:
> Thanks for applying the other patches and for kbd_error()! Now everything
> compiles fine with musl libc.
> 
> However kbd_error() does not behave correctly in the case that the first
> argument is 0 (EXIT_SUCCESS). error() does _not_ terminate the program
> if the first argument is EXIT_SUCCESS. Maybe have in addition a
> kbd_warn() so that the __attribute__((noreturn)) can stay for kbd_error()
> (then the exitnum argument would also become superfluous).

You right. I missed it. Fixed.

> My excuse [1] for changing the error messages (no more program name) was
> because the source is inconsistent. For example init_vt() in vlock/vt.c
> or main() in kbdinfo.c mix fprintf(stderr) and error(). It seems to me
> that error was mainly used as a convenient shorthand for
> fprint(stderr) && exit(). (Having a shorthand makes sense.)

Agree. It will be necessary to bring messages to one format.

> PS: Could you maybe note on the webpage that git.altlinux.org is the
> main git repository and might be ahead of the git.kernel.org repo?

I usually send changes to both the repository.

> [1]: http://lists.altlinux.org/pipermail/kbd/2014-December/000487.html

-- 
Rgrds, legion

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Felix Janda]

Alexey Gladkov wrote:
> 29.04.2015 21:11, Felix Janda пишет:
> > Thanks for applying the other patches and for kbd_error()! Now everything
> > compiles fine with musl libc.
> > 
> > However kbd_error() does not behave correctly in the case that the first
> > argument is 0 (EXIT_SUCCESS). error() does _not_ terminate the program
> > if the first argument is EXIT_SUCCESS. Maybe have in addition a
> > kbd_warn() so that the __attribute__((noreturn)) can stay for kbd_error()
> > (then the exitnum argument would also become superfluous).
> 
> You right. I missed it. Fixed.

Thanks, now the replacement should be equivalent to what it is replacing.

Felix

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

29.04.2015 21:11, Felix Janda пишет:
> For example init_vt() in vlock/vt.c
> or main() in kbdinfo.c mix fprintf(stderr) and error(). It seems to me
> that error was mainly used as a convenient shorthand for
> fprint(stderr) && exit(). (Having a shorthand makes sense.)

I did my homework and reduce the calls of perror and fprint(stderr) &&
exit(). Other calls in the parts that I will replace soon.

-- 
Rgrds, legion

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

04.05.2015 22:51, Felix Janda пишет:
> Спасибо, looks like much more consistent and also more informative
> error reporting.

:))))

> Now that there is also kbd_warning(), it is no longer necessary to let
> kbd_error() treat 0 specially and it can get an __attribute__(noreturn).
> See attached patch.

Good catch!

I prefer to save compatibility with the error() function. It costs
nothing, but it does not require to keep in mind the difference
between two implementations.

-- 
Rgrds, legion

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Felix Janda]

Alexey Gladkov wrote:
> I prefer to save compatibility with the error() function. It costs
> nothing, but it does not require to keep in mind the difference
> between two implementations.

There is a small cost (which might not be relevant for kbd):
The compiler does not know that kbd_error() will not return in most
cases and might emit warnings about possibly uninitialized variables
because of that.

Just to mentoin it...

Felix

Re: [kbd] [PATCH 5/5] Replace error() by fprintf() and perror()

[Alexey Gladkov]

06.05.2015 22:34, Felix Janda пишет:
> Alexey Gladkov wrote:
>> I prefer to save compatibility with the error() function. It costs
>> nothing, but it does not require to keep in mind the difference
>> between two implementations.
> 
> There is a small cost (which might not be relevant for kbd):
> The compiler does not know that kbd_error() will not return in most
> cases and might emit warnings about possibly uninitialized variables
> because of that.
> 
> Just to mentoin it...

Agree. There is no reason to keep this compatibility right now.

-- 
Rgrds, legion