From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:59619) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XX8FX-0001Qw-7A for qemu-devel@nongnu.org; Thu, 25 Sep 2014 08:34:23 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XX8FS-000768-Rf for qemu-devel@nongnu.org; Thu, 25 Sep 2014 08:34:19 -0400 Message-ID: <54240BB1.8020908@redhat.com> Date: Thu, 25 Sep 2014 06:33:53 -0600 From: Eric Blake MIME-Version: 1.0 References: <1411638384-5844-1-git-send-email-arei.gonglei@huawei.com> <1411638384-5844-2-git-send-email-arei.gonglei@huawei.com> In-Reply-To: <1411638384-5844-2-git-send-email-arei.gonglei@huawei.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="81JgiW6MXCgew4nmaMUUA6vfl2XlNEAu6" Subject: Re: [Qemu-devel] [PATCH 1/4] os-posix/win32: convert fprintf/perror to error_report List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: arei.gonglei@huawei.com, qemu-devel@nongnu.org Cc: peter.maydell@linaro.org, weidong.huang@huawei.com, qemu-trivial@nongnu.org, luonengjun@huawei.com, mjt@tls.msk.ru, armbru@redhat.com, peter.huangpeng@huawei.com, pbonzini@redhat.com This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --81JgiW6MXCgew4nmaMUUA6vfl2XlNEAu6 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 09/25/2014 03:46 AM, arei.gonglei@huawei.com wrote: > From: Gonglei >=20 > Signed-off-by: Gonglei > --- > os-posix.c | 34 ++++++++++++++++++---------------- > os-win32.c | 3 ++- > 2 files changed, 20 insertions(+), 17 deletions(-) >=20 > diff --git a/os-posix.c b/os-posix.c > index cb2a7f7..9d5ae70 100644 > --- a/os-posix.c > +++ b/os-posix.c > @@ -39,6 +39,7 @@ > #include "sysemu/sysemu.h" > #include "net/slirp.h" > #include "qemu-options.h" > +#include "qemu/error-report.h" > =20 > #ifdef CONFIG_LINUX > #include > @@ -120,11 +121,11 @@ void os_set_proc_name(const char *s) > /* Could rewrite argv[0] too, but that's a bit more complicated. > This simple way is enough for `top'. */ > if (prctl(PR_SET_NAME, name)) { > - perror("unable to change process name"); > + error_report("unable to change process name"); This loses the value of errno that perror would have displayed. Is that reduction in error message quality intentional? If not, then this is not a trivial conversion; if it is, then your commit message should call it out. > @@ -167,20 +168,20 @@ static void change_process_uid(void) > { > if (user_pwd) { > if (setgid(user_pwd->pw_gid) < 0) { > - fprintf(stderr, "Failed to setgid(%d)\n", user_pwd->pw_gid= ); > + error_report("Failed to setgid(%d)\n", user_pwd->pw_gid); No trailing \n for error_report, please. (You got it right in most of your conversions) > @@ -190,11 +191,11 @@ static void change_root(void) > { > if (chroot_dir) { > if (chroot(chroot_dir) < 0) { > - fprintf(stderr, "chroot failed\n"); > + error_report("chroot failed"); > exit(1); > } > if (chdir("/")) { > - perror("not able to chdir to /"); > + error_report("not able to chdir to /"); Another loss of errno value from perror. > exit(1); > } > } > @@ -224,7 +225,7 @@ void os_daemonize(void) > if (len !=3D 1) > exit(1); > else if (status =3D=3D 1) { > - fprintf(stderr, "Could not acquire pidfile: %s\n", str= error(errno)); > + error_report("Could not acquire pidfile: %s", strerror= (errno)); This code is broken. The earlier 'if (len !=3D 1)' fails to print a message before exiting (not to mention it violates coding style by omitting {}). Then, if we get inside the 'else if (status =3D=3D 1)' conditional, then we KNOW that read() succeeded, and therefore errno is unspecified. Printing strerror(errno) on a random value is NOT helpful. > @@ -267,7 +268,7 @@ void os_setup_post(void) > exit(1); > =20 > if (chdir("/")) { > - perror("not able to chdir to /"); > + error_report("not able to chdir to /"); Another loss of errno reporting. > exit(1); > } > TFR(fd =3D qemu_open("/dev/null", O_RDWR)); > @@ -292,10 +293,11 @@ void os_pidfile_error(void) > if (daemonize) { > uint8_t status =3D 1; > if (write(fds[1], &status, 1) !=3D 1) { > - perror("daemonize. Writing to pipe\n"); > + error_report("daemonize. Writing to pipe"); and another. > @@ -338,7 +340,7 @@ int os_mlock(void) > =20 > ret =3D mlockall(MCL_CURRENT | MCL_FUTURE); > if (ret < 0) { > - perror("mlockall"); > + error_report("mlockall"); and another. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --81JgiW6MXCgew4nmaMUUA6vfl2XlNEAu6 Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg iQEcBAEBCAAGBQJUJAuxAAoJEKeha0olJ0NqWTMIAJDoGT2S/60KSxHZMDQjxBsI dc0HIebaxaaISddnhYXBGqmM+5NK5usBoW9n3Xk+nHtebEikV2hQNkJ6hKGhPDfM 90tPUS+W5ABXmfYsMTGR/lc73fCJGSr1D/LcGKVOMdj8lAjr0BOzpcpVXerZJPXq QyfJItsLvgInGelg8jXec7iuM8h9L+3ICa5iW2mUdJwWJjvwLPFsa5N8HJP72pBI 7iA3bve7PQNN+P0gB/eXNuOpdPfMfyB2SwxNAVuQ6RUmrH/sx1wjgkHOBKBkIoYC hscS+5Rczt6/pXBCQya5o3pn9C9EZm1BRd00zybr/FVqt97GrIlVgDKcakapr8A= =qbjV -----END PGP SIGNATURE----- --81JgiW6MXCgew4nmaMUUA6vfl2XlNEAu6--