error.h implementation

[James Y Knight <jyknight@google.com>]

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

While these glibc-introduced functions are, IMO, ugly and not really a
worthwhile addition to a libc interface, they do get used. So, in the
interests of compatibility with existing code, here's an
implementation...they're standalone, and pretty trivial, so it seems
reasonable to include, despite it being unfortunate that they exist at
all.

The added test case here passes with both musl and glibc.

A question: is the weak_alias stuff required in this case? I'm not
100% clear as to when that's required, and when it's not, but I think
perhaps it's not actually needed here, because nothing else in the
libc refers to these nonstandard symbols, and the file defines no
standard symbols?

[-- Attachment #2: 0001-Add-support-for-the-glibc-specific-error.h-header.patch --]
[-- Type: application/octet-stream, Size: 3685 bytes --]

From b6f37d7555a68392ea976621aa9d408efec75c9e Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight@google.com>
Date: Mon, 22 Jul 2019 20:39:26 -0400
Subject: [PATCH] Add support for the glibc-specific error.h header.

This includes the functions 'error', 'error_at_line', and their
associated global variables.
---
 dynamic.list       |  7 ++++
 include/error.h    | 23 +++++++++++++
 src/legacy/error.c | 80 ++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 110 insertions(+)
 create mode 100644 include/error.h
 create mode 100644 src/legacy/error.c

diff --git a/dynamic.list b/dynamic.list
index ee0d363b..d8f57b85 100644
--- a/dynamic.list
+++ b/dynamic.list
@@ -42,4 +42,11 @@ __progname;
 __progname_full;
 
 __stack_chk_guard;
+
+error_print_progname;
+error_message_count;
+error_one_per_line;
+__error_print_progname;
+__error_message_count;
+__error_one_per_line;
 };
diff --git a/include/error.h b/include/error.h
new file mode 100644
index 00000000..d4e7fad6
--- /dev/null
+++ b/include/error.h
@@ -0,0 +1,23 @@
+#ifndef _ERROR_H
+#define _ERROR_H
+
+#include <features.h>
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+extern void (*error_print_progname) (void);
+extern unsigned int error_message_count;
+extern int error_one_per_line;
+
+void error(int status, int errnum, const char *fmt, ...);
+void error_at_line(int status, int errnum,
+                   const char *file, unsigned int linenum,
+                   const char *fmt, ...);
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif
diff --git a/src/legacy/error.c b/src/legacy/error.c
new file mode 100644
index 00000000..2b820954
--- /dev/null
+++ b/src/legacy/error.c
@@ -0,0 +1,80 @@
+#include <error.h>
+#include <stdarg.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include "stdio_impl.h"
+#include "libc.h"
+
+void (*__error_print_progname) (void) = 0;
+unsigned int __error_message_count = 0;
+int __error_one_per_line = 0;
+
+static unsigned int saved_linenum = 0;
+static const char *saved_file = 0;
+
+static void errorv(int status, int errnum,
+		   const char *file, unsigned int linenum,
+		   const char *fmt, va_list ap) {
+	++__error_message_count;
+
+	fflush(stdout);
+	FLOCK(stderr);
+
+	if (__error_print_progname)
+		__error_print_progname();
+	else {
+		fprintf(stderr, "%s:", __progname_full);
+		if (!file)
+			fputc(' ', stderr);
+	}
+
+	if (file)
+		fprintf(stderr, "%s:%u: ", file, linenum);
+
+	vfprintf(stderr, fmt, ap);
+	if (errnum)
+		fprintf(stderr, ": %s", strerror(errnum));
+	fputc('\n', stderr);
+
+	fflush(stderr);
+	FUNLOCK(stderr);
+
+	if (status)
+		exit(status);
+}
+
+void __error(int status, int errnum, const char *fmt, ...) {
+	va_list ap;
+	va_start(ap, fmt);
+	errorv(status, errnum, NULL, 0, fmt, ap);
+	va_end(ap);
+}
+
+void __error_at_line(int status, int errnum,
+                     const char *file, unsigned int linenum,
+                     const char *fmt, ...) {
+	if (error_one_per_line) {
+		if(saved_linenum == linenum && file != NULL &&
+		   saved_file != NULL && !strcmp(file, saved_file))
+			return;
+		saved_linenum = linenum;
+		// Assuming that the lifetime of the passed in file name extends
+		// until the next call is rather questionable, but appears to be
+		// the expected semantics.
+		saved_file = file;
+	}
+
+	va_list ap;
+	va_start(ap, fmt);
+	errorv(status, errnum, file, linenum, fmt, ap);
+	va_end(ap);
+}
+
+
+weak_alias(__error_print_progname, error_print_progname);
+weak_alias(__error_message_count, error_message_count);
+weak_alias(__error_one_per_line, error_one_per_line);
+
+weak_alias(__error, error);
+weak_alias(__error_at_line, error_at_line);
-- 
2.22.0.657.g960e92d24f-goog


[-- Attachment #3: 0001-Add-tests-for-error.h-functions.patch --]
[-- Type: application/octet-stream, Size: 2341 bytes --]

From 751247d2cdcf3f64aa27da0dc9801169b78abec5 Mon Sep 17 00:00:00 2001
From: James Y Knight <jyknight@google.com>
Date: Mon, 22 Jul 2019 22:57:39 -0400
Subject: [PATCH] Add tests for <error.h> functions.

---
 src/functional/error.c | 68 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)
 create mode 100644 src/functional/error.c

diff --git a/src/functional/error.c b/src/functional/error.c
new file mode 100644
index 0000000..e0b063c
--- /dev/null
+++ b/src/functional/error.c
@@ -0,0 +1,68 @@
+#include <error.h>
+
+#include <errno.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/wait.h>
+#include <unistd.h>
+#include "test.h"
+
+#define ASSERT(c) do { \
+	errno = 0; \
+	if (!(c)) { \
+		t_error("%s failed (errno: %s)\n", #c, strerror(errno)); \
+		exit(1); \
+	} \
+} while(0)
+
+void my_print_progname(void) {
+	fputs("Progname:", stderr);
+}
+
+const char *expected_output =
+		"src/functional/error.exe: Test1\n"
+		"src/functional/error.exe:File:44: Test2 hello\n"
+		"Progname:File:44: Test3 4\n"
+		"Progname:Test4 hello: No such file or directory\n"
+		"Progname:OtherFile:44: Test6\n"
+		"Progname:OtherFile:47: Test7\n"
+		"error_message_count:6\n"
+		"Progname:Last error\n";
+
+int main() {
+        int fd, pid, status;
+	int pipefds[2];
+	ASSERT(pipe(pipefds) == 0);
+
+	ASSERT((pid = fork()) >= 0);
+	if (pid == 0) {
+		ASSERT(dup2(pipefds[1], 1) == 1);
+		ASSERT(dup2(pipefds[1], 2) == 2);
+
+		error(0, 0, "Test1");
+		error_at_line(0, 0, "File", 44, "Test2 %s", "hello");
+
+		error_print_progname = my_print_progname;
+
+		error_one_per_line = 1;
+		error_at_line(0, 0, "File", 44, "Test3 %d", 4);
+		error(0, ENOENT, "Test4 %s", "hello");
+		error_at_line(0, 0, "File", 44, "Test5 (skipped)");
+		error_at_line(0, 0, "OtherFile", 44, "Test6");
+		error_at_line(0, 0, "OtherFile", 47, "Test7");
+		printf("error_message_count:%d\n", error_message_count);
+		error(77, 0, "Last error");
+	}
+	ASSERT(waitpid(pid, &status, 0) == pid);
+	ASSERT(WIFEXITED(status) && WEXITSTATUS(status) == 77);
+
+	char buf[1000];
+	ssize_t count;
+	ASSERT((count = read(pipefds[0], buf, sizeof(buf)-1)) >= 0);
+	buf[count] = 0;
+	if (strcmp(buf, expected_output)) {
+		t_error("Unexpected output. Got:\n%s\n", buf);
+	}
+	return t_status;
+}
-- 
2.22.0.657.g960e92d24f-goog