[libvirt] RFC: Removing use of strerror() in favour of strerror_r()
Dave Allan
dallan at redhat.com
Thu Dec 11 18:22:19 UTC 2008
Daniel P. Berrange wrote:
> The man page for 'strerror()' has this to say
>
> The strerror() function returns a string describ-
> ing the error code passed in the argument errnum,
> possibly using the LC_MESSAGES part of the current
> locale to select the appropriate language. This
> string must not be modified by the application,
> but may be modified by a subsequent call to per-
> ror(3) or strerror(). No library function will
> modify this string.
>
> ie, strerror is not even remotely safe to use if the application calling
> libvirt uses threads, regardless of whether libvirt itself is threaded.
>
> The replacement is strerror_r(), but it is kind of tedious to use as
> you can't simply pass its return value straight in as format arg.
>
> While looking at this I also noticed that we're rather inconsistent about
> whether we use VIR_ERR_INTERNAL_ERROR or VIR_ERR_SYSTEM_ERROR code for
> reporting system errors. So I'm intending to create a convenient function
> for reporting system errors that hides the sterrror_r horribleness and
> also standardizes on error code VIR_ERR_SYSTEM_ERROR. At the same time
> also creating a convenience function for reporting OOM, since we have
> wildly inconsistent reporting of that too.
>
> The API contracts for src/virterror_internal.h are intended to be:
>
> void virReportSystemErrorFull(virConnectPtr conn,
> int domcode,
> int theerrno,
> const char *filename,
> const char *funcname,
> long long linenr,
> const char *fmt, ...)
> ATTRIBUTE_FORMAT(printf, 7, 8);
>
> void virReportOOMFull(virConnectPtr conn,
> int domcode,
> const char *filename,
> const char *funcname,
> long long linenr);
>
>
> And using a macro to automatically set domcode, filename, funcname
> and linenr, since its tedious to make the caller supply all those
>
>
> #define virReportSystemError(conn, theerrno, fmt,...) \
> virReportSystemErrorFull((conn), \
> VIR_FROM_THIS, \
> (theerrno), \
> __FILE__, __FUNCTION__, __LINE__, \
> (fmt), __VA_ARGS__)
>
> #define virReportOOM(conn) \
> virReportOOMFull((conn), VIR_FROM_THIS, \
> __FILE__, __FUNCTION__, __LINE__) \
>
> So whereas before you might do
>
> xenXMError (conn, VIR_ERR_INTERNAL_ERROR,
> _("cannot stat %s: %s"),
> filename, strerror(errno));
>
> xenXMError (conn, VIR_ERR_NO_MEMORY, "%s", strerror(errno));
>
>
> Now you would do:
>
> virReportSystemError(conn, errno,
> _("cannot stat: %s"),
> filename);
>
> virReportOOM(conn);
>
> Before I actually go and start making this change across all 300 uses
> of strerror(), do people agree with this approach....
>
> Daniel
That seems like a good approach to me. In general, I think anything
that reduces the number of arguments that the caller must pass to
reporting fuctions is a good thing.
Dave
More information about the libvir-list
mailing list