[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 13/15] tap-solaris: Convert tap_open() to Error
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 13/15] tap-solaris: Convert tap_open() to Error |
Date: |
Fri, 15 May 2015 10:48:38 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) |
Eric Blake <address@hidden> 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 <address@hidden>
>> ---
>> 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 <address@hidden>
Thanks!
- Re: [Qemu-devel] [PATCH 10/15] tap: Permit incremental conversion of tap_open() to Error, (continued)
- [Qemu-devel] [PATCH 12/15] tap-bsd: Convert tap_open() to Error, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 01/15] net: Improve error message for -net hubport a bit, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 09/15] tap: Convert launch_script() to Error, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 13/15] tap-solaris: Convert tap_open() to Error, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 02/15] net: Permit incremental conversion of init functions to Error, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 15/15] tap: Improve -netdev/netdev_add/-net/... tap error reporting, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 14/15] tap: Finish conversion of tap_open() to Error, Markus Armbruster, 2015/05/12
- [Qemu-devel] [PATCH 08/15] tap: Convert net_init_tap_one() to Error, Markus Armbruster, 2015/05/12