[ovs-dev] [PATCH] util: Make out_of_memory() call abort() instead of exit(EXIT_FAILURE).

Ben Pfaff blp at nicira.com
Thu Feb 24 00:44:33 UTC 2011


Thanks.  I pushed this.

On Wed, Feb 23, 2011 at 03:54:15PM -0800, Ethan Jackson wrote:
> Looks Good.
> 
> On Wed, Feb 23, 2011 at 3:43 PM, Ben Pfaff <blp at nicira.com> wrote:
> > exit(EXIT_FAILURE) will make a monitoring process (the one created by
> > --monitor) think that it should exit.  But the most likely reason for
> > out_of_memory() to be called is a bug: probably, the process is trying
> > to allocate more memory than there is available address space, e.g.
> > something like malloc(-1).  So it's better, in my opinion, to call abort()
> > instead, so that the monitor process restarts the daemon and we are more
> > likely to stay alive and, in addition, get a core dump and a useful bug
> > report.
> >
> > I decided to implement a new general-purpose function for this purpose in
> > case we run into other similar situations in the future.
> >
> > (I haven't actually run into this problem in practice.  This commit is
> > just speculation about what is better behavior.)
> > ---
> >  lib/util.c |   55 ++++++++++++++++++++++++++++++++++++++++++++++---------
> >  lib/util.h |    4 ++++
> >  2 files changed, 50 insertions(+), 9 deletions(-)
> >
> > diff --git a/lib/util.c b/lib/util.c
> > index f784f03..b9dbdc4 100644
> > --- a/lib/util.c
> > +++ b/lib/util.c
> > @@ -34,7 +34,7 @@ const char *program_name;
> >  void
> >  out_of_memory(void)
> >  {
> > -    ovs_fatal(0, "virtual memory exhausted");
> > +    ovs_abort(0, "virtual memory exhausted");
> >  }
> >
> >  void *
> > @@ -171,32 +171,69 @@ ovs_strzcpy(char *dst, const char *src, size_t size)
> >     }
> >  }
> >
> > +/* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' is
> > + * nonzero, then it is formatted with ovs_retval_to_string() and appended to
> > + * the message inside parentheses.  Then, terminates with abort().
> > + *
> > + * This function is preferred to ovs_fatal() in a situation where it would make
> > + * sense for a monitoring process to restart the daemon.
> > + *
> > + * 'format' should not end with a new-line, because this function will add one
> > + * itself. */
> > +void
> > +ovs_abort(int err_no, const char *format, ...)
> > +{
> > +    va_list args;
> > +
> > +    va_start(args, format);
> > +    ovs_error_valist(err_no, format, args);
> > +    va_end(args);
> > +
> > +    abort();
> > +}
> > +
> > +/* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' is
> > + * nonzero, then it is formatted with ovs_retval_to_string() and appended to
> > + * the message inside parentheses.  Then, terminates with EXIT_FAILURE.
> > + *
> > + * 'format' should not end with a new-line, because this function will add one
> > + * itself. */
> >  void
> >  ovs_fatal(int err_no, const char *format, ...)
> >  {
> >     va_list args;
> >
> > -    fprintf(stderr, "%s: ", program_name);
> >     va_start(args, format);
> > -    vfprintf(stderr, format, args);
> > +    ovs_error_valist(err_no, format, args);
> >     va_end(args);
> > -    if (err_no != 0)
> > -        fprintf(stderr, " (%s)", ovs_retval_to_string(err_no));
> > -    putc('\n', stderr);
> >
> >     exit(EXIT_FAILURE);
> >  }
> >
> > +/* Prints 'format' on stderr, formatting it like printf() does.  If 'err_no' is
> > + * nonzero, then it is formatted with ovs_retval_to_string() and appended to
> > + * the message inside parentheses.
> > + *
> > + * 'format' should not end with a new-line, because this function will add one
> > + * itself. */
> >  void
> >  ovs_error(int err_no, const char *format, ...)
> >  {
> > -    int save_errno = errno;
> >     va_list args;
> >
> > -    fprintf(stderr, "%s: ", program_name);
> >     va_start(args, format);
> > -    vfprintf(stderr, format, args);
> > +    ovs_error_valist(err_no, format, args);
> >     va_end(args);
> > +}
> > +
> > +/* Same as ovs_error() except that the arguments are supplied as a va_list. */
> > +void
> > +ovs_error_valist(int err_no, const char *format, va_list args)
> > +{
> > +    int save_errno = errno;
> > +
> > +    fprintf(stderr, "%s: ", program_name);
> > +    vfprintf(stderr, format, args);
> >     if (err_no != 0) {
> >         fprintf(stderr, " (%s)", ovs_retval_to_string(err_no));
> >     }
> > diff --git a/lib/util.h b/lib/util.h
> > index e741067..635d331 100644
> > --- a/lib/util.h
> > +++ b/lib/util.h
> > @@ -135,9 +135,13 @@ void *x2nrealloc(void *p, size_t *n, size_t s);
> >  void ovs_strlcpy(char *dst, const char *src, size_t size);
> >  void ovs_strzcpy(char *dst, const char *src, size_t size);
> >
> > +void ovs_abort(int err_no, const char *format, ...)
> > +    PRINTF_FORMAT(2, 3) NO_RETURN;
> >  void ovs_fatal(int err_no, const char *format, ...)
> >     PRINTF_FORMAT(2, 3) NO_RETURN;
> >  void ovs_error(int err_no, const char *format, ...) PRINTF_FORMAT(2, 3);
> > +void ovs_error_valist(int err_no, const char *format, va_list)
> > +    PRINTF_FORMAT(2, 0);
> >  const char *ovs_retval_to_string(int);
> >  void ovs_hex_dump(FILE *, const void *, size_t, uintptr_t offset, bool ascii);
> >
> > --
> > 1.7.2.3
> >
> >
> > _______________________________________________
> > dev mailing list
> > dev at openvswitch.org
> > http://openvswitch.org/mailman/listinfo/dev_openvswitch.org
> >




More information about the dev mailing list