Re: [Qemu-devel] [PATCH 21/38] ivshmem: Disentangle ivshmem_read()

[Markus Armbruster]

Marc-André Lureau <address@hidden> writes:

> On Wed, Mar 2, 2016 at 4:53 PM, Markus Armbruster <address@hidden> wrote:
>>>> +    if (msg == -1) {
>>>> +        process_msg_shmem(s, fd);
>>>
>>> the previous code used to close fd if any, it's worth to keep that imho
>>
>> I'm blind.  Where?
>
> Sorry, wrong place I looked at, seems you got them all.
>
>     if (msg < -1 || msg > IVSHMEM_MAX_PEERS) {
>         error_report("server sent invalid message %" PRId64, msg);
>         close(fd);
>         return;
>     }
>
>
> However, why not keep the if fd != -1 here (not a great idea to call
> close otherwise)

We refuse to make the code more verbose to avoid free(NULL), and I very
much agree with that.

close(-1) is like free(NULL) in that it is perfectly safe.  Where they
differ is performance: free() checks for null right away, but close()
checks only after switching to supervisor mode.  Doesn't matter on an
error path.