Eric Blake <ebl...@redhat.com> writes: > On 05/12/2015 06:03 AM, Markus Armbruster wrote: >> Fixes inappropriate use of syslog(). >> >> Not fixed: leaks on error paths, suspicious non-fatal errors. FIXMEs >> added instead. > > At least you're admitting where the code is still bad.
Actually, git-rm felt pretty tempting. >> Signed-off-by: Markus Armbruster <arm...@redhat.com> >> --- >> net/tap-solaris.c | 59 >> ++++++++++++++++++++++++++++--------------------------- >> 1 file changed, 30 insertions(+), 29 deletions(-) >> > >> @@ -99,20 +100,20 @@ static int tap_alloc(char *dev, size_t dev_size) >> strioc_ppa.ic_len = sizeof(ppa); >> strioc_ppa.ic_dp = (char *)&ppa; >> if ((ppa = ioctl (tap_fd, I_STR, &strioc_ppa)) < 0) >> - syslog (LOG_ERR, "Can't assign new interface"); >> + error_report("Can't assign new interface"); > > I see you're fixing spacing while at it, as well. > >> >> TFR(if_fd = open("/dev/tap", O_RDWR, 0)); >> if (if_fd < 0) { >> - syslog(LOG_ERR, "Can't open /dev/tap (2)"); >> - return -1; >> + error_setg(errp, "Can't open /dev/tap (2)"); >> + return -1; >> } >> if(ioctl(if_fd, I_PUSH, "ip") < 0){ >> - syslog(LOG_ERR, "Can't push IP module"); > > Should you add the space after 'if' while touching this? Fixing up just enough to make checkpatch happy. Also keeps git-blame useful even without -w. >> - return -1; >> + error_setg(errp, "Can't push IP module"); >> + return -1; >> } >> >> if (ioctl(if_fd, SIOCGLIFFLAGS, &ifr) < 0) >> - syslog(LOG_ERR, "Can't get flags\n"); >> + error_report("Can't get flags"); > > What about adding missing {} while touching this file? Hmm - there's > enough cruft that it may involve a separate patch just to clean up > style. For this patch, I'm not going to hold up review on style. > > Reviewed-by: Eric Blake <ebl...@redhat.com> Thanks!