On Thu, Mar 24, 2011 at 12:42 PM, Kevin Wolf <kw...@redhat.com> wrote:
> Am 23.03.2011 21:50, schrieb Stefan Hajnoczi:
>> On Wed, Mar 23, 2011 at 8:27 PM, Juan Quintela <quint...@redhat.com> wrote:
>>> Stefan Hajnoczi <stefa...@linux.vnet.ibm.com> wrote:
>>>> +
>>>> + if (s->fd == -1) {
>>>> + s->fd = qemu_open(bs->filename, s->open_flags, 0644);
>>>
>>> Everything else on that file uses plain "open" not "qemu_open".
>>> diference is basically that qemu_open() adds flag O_CLOEXEC.
>>>
>>> I don't know if this one should be vanilla open or the other ones
>>> qemu_open().
>>>
>>> What do you think?
>>
>> raw_open_common() uses qemu_open(). That's why I used it.
>
> And I think it's correct. There's no reason not to set O_CLOEXEC here.
> Maybe some of the open() users need to be fixed.
>
>>>> + if (s->fd < 0) {
>>>> + return 0;
>>>> + }
>>>> + }
>>>> +
>>>> + ret = (ioctl(s->fd, CDROM_DRIVE_STATUS, CDSL_CURRENT) == CDS_DISC_OK);
>>>
>>> parens are not needed around ==.
>>
>> Yes, if you want I'll remove them. I just did it for readability.
>
> I like them.
I will have to #ifdef QUINTELA and #ifdef KWOLF :). Seriously, I'll
leave the braces unless anyone feels really strongly either way. This
passes checkpatch.pl BTW.
Stefan
