[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH] logging: confirm that we want to ignore a write error



On Mon, Jan 18, 2010 at 02:56:36PM +0100, Jim Meyering wrote:
> Ignoring a write failure is a big deal, so when you do that deliberately,
> it's worth marking in such a way that code analyzers don't report a
> false positive.  Until a year or so ago, people thought using (void)
> would be enough.  But newer gcc warns in spite of that (for functions
> marked with the warn_unused_result attribute), so now we use the
> ignore_value function from gnulib.
> 
> A coverity warning prompted the change below, but if we declare
> saferead with the warn_unused_result attribute, gcc would, too.
> 
> >From 62d2c7f69608407afa9a4bf38f0d29ea3eec88f5 Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering redhat com>
> Date: Mon, 18 Jan 2010 11:51:01 +0100
> Subject: [PATCH] logging: confirm that we want to ignore a write error
> 
> * src/util/logging.c (virLogMessage): Include "ignore-value.h".
> Use it to ignore the return value of safewrite.
> Use STDERR_FILENO, rather than "2".
> * bootstrap (modules): Add ignore-value.
> * gnulib: Update to latest, for ignore-value that is now LGPLv2+.
> ---
>  .gnulib            |    2 +-
>  bootstrap          |    1 +
>  src/util/logging.c |    5 +++--
>  3 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/.gnulib b/.gnulib
> index 4c52807..146d914 160000
> --- a/.gnulib
> +++ b/.gnulib
> @@ -1 +1 @@
> -Subproject commit 4c52807f41f238cf0e352317b2dc54f9ba0f0c4f
> +Subproject commit 146d9145073e62a2096a2d6b33f75e93908fedf3
> diff --git a/bootstrap b/bootstrap
> index c07d851..aec5d05 100755
> --- a/bootstrap
> +++ b/bootstrap
> @@ -76,6 +76,7 @@ getpass
>  gettext
>  gitlog-to-changelog
>  gnumakefile
> +ignore-value
>  inet_pton
>  ioctl
>  maintainer-makefile
> diff --git a/src/util/logging.c b/src/util/logging.c
> index 6bd8469..3b3c309 100644
> --- a/src/util/logging.c
> +++ b/src/util/logging.c
> @@ -1,7 +1,7 @@
>  /*
>   * logging.c: internal logging and debugging
>   *
> - * Copyright (C) 2008 Red Hat, Inc.
> + * Copyright (C) 2008, 2010 Red Hat, Inc.
>   *
>   * This library is free software; you can redistribute it and/or
>   * modify it under the terms of the GNU Lesser General Public
> @@ -34,6 +34,7 @@
>  #include <syslog.h>
>  #endif
> 
> +#include "ignore-value.h"
>  #include "logging.h"
>  #include "memory.h"
>  #include "util.h"
> @@ -579,7 +580,7 @@ void virLogMessage(const char *category, int priority, const char *funcname,
>                                 msg, len, virLogOutputs[i].data);
>      }
>      if ((virLogNbOutputs == 0) && (flags != 1))
> -        safewrite(2, msg, len);
> +        ignore_value (safewrite(STDERR_FILENO, msg, len));
>      virLogUnlock();
> 
>      VIR_FREE(msg);
> --

ACK, took me a while to understand why 'ignore_value' works, but it is
worth it. We should add ATTRIBUTE_RETURN_CHECK to safewrite() sometime
soon too.

Daniel
-- 
|: Red Hat, Engineering, London   -o-   http://people.redhat.com/berrange/ :|
|: http://libvirt.org  -o-  http://virt-manager.org  -o-  http://ovirt.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: GnuPG: 7D3B9505  -o-  F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]