git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 1/4] error: save and restore errno

From
Stefan Beller <sbeller@google.com>
Date
Nov 19, 2014, 01:41 UTC
Message-ID
<CAGZ79kbNatX51P3kocndyR+UVeACQR_u3OhkZiJmZrq_UBoJxg@mail.gmail.com>
In-Reply-To
<20141119013710.GA2135@peff.net>

This one makes my day. A really good fix as it minimizes maintenance burden for checking incoming patches for that pattern.

Reviewed-by: Stefan Beller <sbeller@google.com>
On Tue, Nov 18, 2014 at 5:37 PM, Jeff King <peff@peff.net> wrote:
Show 52 quoted lines
> It's common to use error() to return from a function, like:
>
>         if (open(...) < 0)
>                 return error("open failed");
>
> Unfortunately this may clobber the errno from the open()
> call. So we often end up with code like this:
>
>         if (open(...) < 0) {
>                 int saved_errno = errno;
>                 error("open failed");
>                 errno = saved_errno;
>                 return -1;
>         }
>
> which is less nice. Let's teach error() to save and restore
> errno in each call, so that the original errno is preserved.
> This is slightly less efficient for callers which do not
> care, but error code paths are generally not performance
> critical anyway.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
> It's pretty minor to just handle errno in the callers, but I feel like
> I've wanted this at least a dozen times, and it seems like it cannot
> possibly hurt (i.e., I imagine there are callers where we _should_ be
> doing the errno dance but have not realized it).
>
>  usage.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/usage.c b/usage.c
> index ed14645..ee44d57 100644
> --- a/usage.c
> +++ b/usage.c
> @@ -142,10 +142,13 @@ void NORETURN die_errno(const char *fmt, ...)
>  int error(const char *err, ...)
>  {
>         va_list params;
> +       int saved_errno = errno;
>
>         va_start(params, err);
>         error_routine(err, params);
>         va_end(params);
> +
> +       errno = saved_errno;
>         return -1;
>  }
>
> --
> 2.1.2.596.g7379948
>
Previous: Jeff KingNext: Jonathan Nieder
Message 6 of 24 in “refs.c: handle locking failure during transaction better”
  1. refs.c: handle locking failure during transaction betterStefan Beller, Nov 18, 2014
  2. Stefan BellerNov 18, 2014
  3. refs.c: handle locking failure during transaction betterStefan Beller, Nov 19, 2014
  4. 0/4 error cleanups in lock_ref_sha1_basicJeff King, Nov 19, 2014
  5. 1/4 error: save and restore errnoJeff King, Nov 19, 2014
  6. Stefan BellerNov 19, 2014
  7. Jonathan NiederNov 19, 2014
  8. Jeff KingNov 19, 2014
  9. Junio C HamanoNov 19, 2014
  10. Jeff KingNov 19, 2014
  11. 2/4 lock_ref_sha1_basic: simplify errno handlingJeff King, Nov 19, 2014
  12. Jonathan NiederNov 19, 2014
  13. Michael HaggertyNov 21, 2014
  14. 3/4 lock_ref_sha1_basic: simplify error code pathJeff King, Nov 19, 2014
  15. Jonathan NiederNov 19, 2014
  16. Jeff KingNov 19, 2014
  17. Jonathan NiederNov 19, 2014
  18. Junio C HamanoNov 19, 2014
  19. Jeff KingNov 19, 2014
  20. Junio C HamanoNov 19, 2014
  21. Jeff KingNov 19, 2014
  22. Jonathan NiederNov 20, 2014
  23. 4/4 lock_ref_sha1_basic: do not die on locking errorsJeff King, Nov 19, 2014
  24. Jonathan NiederNov 19, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.