[libvirt] RFC: Removing use of strerror() in favour of strerror_r()

[Daniel P. Berrange]

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
-- 
|: 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 :|