From 5cddde1fb2b8466d8104595008eafabd0728de5d Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 31 Jul 2017 11:30:55 +0100 Subject: Consistently save errno immediately after the operation setting it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Prevent the situation where errno is set by function A, then function B is called (which is typically _(), but could be anything else) and it overwrites errno, then errno is checked by the caller. errno is a horrific API, and we need to be careful to save its value as soon as a function call (which might set it) returns. i.e. Follow the pattern: int errsv, ret; ret = some_call_which_might_set_errno (); errsv = errno; if (ret < 0) puts (strerror (errsv)); This patch implements that pattern throughout GLib. There might be a few places in the test code which still use errno directly. They should be ported as necessary. It doesn’t modify all the call sites like this: if (some_call_which_might_set_errno () && errno == ESOMETHING) since the refactoring involved is probably more harmful than beneficial there. It does, however, refactor other call sites regardless of whether they were originally buggy. https://bugzilla.gnome.org/show_bug.cgi?id=785577 --- tests/gobject/timeloop-closure.c | 8 +++++--- tests/mainloop-test.c | 3 ++- tests/spawn-test-win32-gui.c | 9 ++++++--- tests/spawn-test.c | 15 ++++++++++----- tests/testglib.c | 7 +++++-- tests/timeloop-basic.c | 8 +++++--- tests/timeloop.c | 8 +++++--- 7 files changed, 38 insertions(+), 20 deletions(-) (limited to 'tests') diff --git a/tests/gobject/timeloop-closure.c b/tests/gobject/timeloop-closure.c index 9de00d32f..c904c2a45 100644 --- a/tests/gobject/timeloop-closure.c +++ b/tests/gobject/timeloop-closure.c @@ -23,7 +23,8 @@ io_pipe (GIOChannel **channels) if (pipe(fds) < 0) { - fprintf (stderr, "Cannot create pipe %s\n", g_strerror (errno)); + int errsv = errno; + fprintf (stderr, "Cannot create pipe %s\n", g_strerror (errsv)); exit (1); } @@ -134,7 +135,7 @@ input_callback (GIOChannel *source, static void create_child (void) { - int pid; + int pid, errsv; GIOChannel *in_channels[2]; GIOChannel *out_channels[2]; GSource *source; @@ -143,6 +144,7 @@ create_child (void) io_pipe (out_channels); pid = fork (); + errsv = errno; if (pid > 0) /* Parent */ { @@ -172,7 +174,7 @@ create_child (void) } else /* Error */ { - fprintf (stderr, "Cannot fork: %s\n", g_strerror (errno)); + fprintf (stderr, "Cannot fork: %s\n", g_strerror (errsv)); exit (1); } } diff --git a/tests/mainloop-test.c b/tests/mainloop-test.c index 5d305508e..9b8386a65 100644 --- a/tests/mainloop-test.c +++ b/tests/mainloop-test.c @@ -202,7 +202,8 @@ io_pipe (GIOChannel **channels) if (pipe(fds) < 0) { - g_warning ("Cannot create pipe %s\n", g_strerror (errno)); + int errsv = errno; + g_warning ("Cannot create pipe %s\n", g_strerror (errsv)); exit (1); } diff --git a/tests/spawn-test-win32-gui.c b/tests/spawn-test-win32-gui.c index 45529d011..2798ce6da 100644 --- a/tests/spawn-test-win32-gui.c +++ b/tests/spawn-test-win32-gui.c @@ -60,7 +60,8 @@ WinMain (struct HINSTANCE__ *hInstance, if (write (outfd, &n, sizeof (n)) == -1 || write (outfd, "Hello there", n) == -1) { - sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errno)); + int errsv = errno; + sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errsv)); MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); exit (1); } @@ -81,8 +82,9 @@ WinMain (struct HINSTANCE__ *hInstance, if ((k = read (infd, buf, n)) != n) { + int errsv = errno; if (k == -1) - sprintf (buf, "spawn-test-win32-gui: Read: %s", strerror (errno)); + sprintf (buf, "spawn-test-win32-gui: Read: %s", strerror (errsv)); else sprintf (buf, "spawn-test-win32-gui: Got only %d bytes", k); MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); @@ -96,7 +98,8 @@ WinMain (struct HINSTANCE__ *hInstance, if (write (outfd, &n, sizeof (n)) == -1 || write (outfd, "See ya", n) == -1) { - sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errno)); + int errsv = errno; + sprintf (buf, "spawn-test-win32-gui: Write: %s", strerror (errsv)); MessageBox (NULL, buf, lpszCmdLine, MB_ICONERROR|MB_SYSTEMMODAL); exit (1); } diff --git a/tests/spawn-test.c b/tests/spawn-test.c index 217cfd941..05cce17f8 100644 --- a/tests/spawn-test.c +++ b/tests/spawn-test.c @@ -232,8 +232,9 @@ run_tests (void) if ((k = read (pipeup[0], &n, sizeof (n))) != sizeof (n)) { + int errsv = errno; if (k == -1) - fprintf (stderr, "Read error: %s\n", g_strerror (errno)); + fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", sizeof (n), k); @@ -242,8 +243,9 @@ run_tests (void) if ((k = read (pipeup[0], buf, n)) != n) { + int errsv = errno; if (k == -1) - fprintf (stderr, "Read error: %s\n", g_strerror (errno)); + fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", n, k); @@ -254,14 +256,16 @@ run_tests (void) if (write (pipedown[1], &n, sizeof (n)) == -1 || write (pipedown[1], "Bye then", n) == -1) { - fprintf (stderr, "Write error: %s\n", g_strerror (errno)); + int errsv = errno; + fprintf (stderr, "Write error: %s\n", g_strerror (errsv)); exit (1); } if ((k = read (pipeup[0], &n, sizeof (n))) != sizeof (n)) { + int errsv = errno; if (k == -1) - fprintf (stderr, "Read error: %s\n", g_strerror (errno)); + fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", sizeof (n), k); @@ -270,8 +274,9 @@ run_tests (void) if ((k = read (pipeup[0], buf, n)) != n) { + int errsv = errno; if (k == -1) - fprintf (stderr, "Read error: %s\n", g_strerror (errno)); + fprintf (stderr, "Read error: %s\n", g_strerror (errsv)); else fprintf (stderr, "Wanted to read %d bytes, got %d\n", n, k); diff --git a/tests/testglib.c b/tests/testglib.c index e9f7f36bc..b5a833178 100644 --- a/tests/testglib.c +++ b/tests/testglib.c @@ -907,6 +907,7 @@ test_file_functions (void) char template[32]; char *name_used, chars[62]; gint fd, n; + int errsv; strcpy (template, "foobar"); fd = g_mkstemp (template); @@ -919,15 +920,17 @@ test_file_functions (void) if (fd == -1) g_error ("g_mkstemp didn't work for template %s\n", template); n = write (fd, hello, hellolen); + errsv = errno; if (n == -1) - g_error ("write() failed: %s\n", g_strerror (errno)); + g_error ("write() failed: %s\n", g_strerror (errsv)); else if (n != hellolen) g_error ("write() should have written %d bytes, wrote %d\n", hellolen, n); lseek (fd, 0, 0); n = read (fd, chars, sizeof (chars)); + errsv = errno; if (n == -1) - g_error ("read() failed: %s\n", g_strerror (errno)); + g_error ("read() failed: %s\n", g_strerror (errsv)); else if (n != hellolen) g_error ("read() should have read %d bytes, got %d\n", hellolen, n); diff --git a/tests/timeloop-basic.c b/tests/timeloop-basic.c index 2c11bc506..7f952263a 100644 --- a/tests/timeloop-basic.c +++ b/tests/timeloop-basic.c @@ -25,7 +25,8 @@ my_pipe (int *fds) { if (pipe(fds) < 0) { - fprintf (stderr, "Cannot create pipe %s\n", strerror (errno)); + int errsv = errno; + fprintf (stderr, "Cannot create pipe %s\n", strerror (errsv)); exit (1); } } @@ -121,7 +122,7 @@ input_callback (int source, int dest) void create_child (int pos) { - int pid; + int pid, errsv; int in_fds[2]; int out_fds[2]; @@ -129,6 +130,7 @@ create_child (int pos) my_pipe (out_fds); pid = fork (); + errsv = errno; if (pid > 0) /* Parent */ { @@ -150,7 +152,7 @@ create_child (int pos) } else /* Error */ { - fprintf (stderr,"Cannot fork: %s\n", strerror (errno)); + fprintf (stderr,"Cannot fork: %s\n", strerror (errsv)); exit (1); } } diff --git a/tests/timeloop.c b/tests/timeloop.c index a1f69ef6b..8b5aa3641 100644 --- a/tests/timeloop.c +++ b/tests/timeloop.c @@ -22,7 +22,8 @@ io_pipe (GIOChannel **channels) if (pipe(fds) < 0) { - fprintf (stderr, "Cannot create pipe %s\n", g_strerror (errno)); + int errsv = errno; + fprintf (stderr, "Cannot create pipe %s\n", g_strerror (errsv)); exit (1); } @@ -136,7 +137,7 @@ input_callback (GIOChannel *source, static void create_child (void) { - int pid; + int pid, errsv; GIOChannel *in_channels[2]; GIOChannel *out_channels[2]; @@ -144,6 +145,7 @@ create_child (void) io_pipe (out_channels); pid = fork (); + errsv = errno; if (pid > 0) /* Parent */ { @@ -166,7 +168,7 @@ create_child (void) } else /* Error */ { - fprintf (stderr, "Cannot fork: %s\n", g_strerror (errno)); + fprintf (stderr, "Cannot fork: %s\n", g_strerror (errsv)); exit (1); } } -- cgit v1.2.3-70-g09d2