[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 6 Aug 2019 18:06:12 -0400
From: James Y Knight <jyknight@...gle.com>
To: musl@...ts.openwall.com
Subject: Re: error.h implementation
On Tue, Aug 6, 2019, 12:13 PM Rich Felker <dalias@...c.org> wrote:
> > diff --git a/src/legacy/error.c b/src/legacy/error.c
> > new file mode 100644
> > index 00000000..c5000fa4
> > --- /dev/null
> > +++ b/src/legacy/error.c
> > @@ -0,0 +1,69 @@
> > +#include <error.h>
> > +#include <stdarg.h>
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.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);
> > + flockfile(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);
> > +
> > + funlockfile(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);
> > +}
> > --
> > 2.22.0.709.g102302147b-goog
>
> One other question: what is the thread-safety of these interfaces
> supposed to be? The globals are accessed without any synchronization,
> but flockfile is used to synchronize with concurrent access to stderr.
> This is plausibly correct (if the error.h functions themselves can't
> be used from multiple threads, but can be used in a multithreaded
> application), but at least questionable.
>
> The error_at_line stuff is probably not possible to make thread-safe
> without thread-local state for saved_file/linenum, which would be a
> disproportionate cost for these interfaces.
>
> I'm not objecting to anything with these comments, just wanting to
> confirm that you think it's reasonable as-is.
>
Hmm, good question! I expect most uses are single threaded but it's better
not to be thread-unsafe.
So, I think it's probably best to move everything within the stderr lock
(which means moving the flockfile out to the error and error_at_line
functions). That at least makes these safe w.r.t. themselves. Users
accessing the public globals will still have to take care, which is
unavoidable and fine.
I just noticed this is still using '__progname_full' instead of the
public 'program_invocation_name',
which I should also fix.
(But I'm out and won't be able to update anything for a couple weeks,
sorry!)
>
Content of type "text/html" skipped
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
