Re: [PATCH v3] xml: introduce startupPolicy for chardev device

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Eric,

> >> +        dev->source.data.file.path = strdup("/dev/null");
> >> +
> >> +        if (!(dev->source.data.file.path)) {
> >> +            virReportOOMError();
> >> +            return -1;
> >> +        }
> >> +
> >
> > This can be reduced to:
> > if (VIR_STRDUP(dev->source.data.file.path, "/dev/null") < 0)
> >     return -1;
> 
> That, and 'make syntax-check' should have warned you that use of
> strdup() is forbidden.
> 

I will test the 'make syntax-check' in next update.

> >
> >> +        if ((fd = open(dev->source.data.file.path,
> >> +                       O_CREAT | O_APPEND, S_IRUSR|S_IWUSR)) < 0) {
> >> +            virReportSystemError(errno,
> >> +                                 _("Unable to pre-create chardev file '%s'"),
> >> +                                 dev->source.data.file.path);
> >> +            return -1;
> >> +        }
> >>      }
> >
> > I think we can safely assume /dev/null to exist on any system where QEMU runs
> > and there's no need to pre-create it.
> 
> Correct assumption.  (Even though we don't currently compile qemu
> support on mingw, we use gnulib, which guarantees that open("/dev/null")
> will do the right thing on mingw even though that is the one platform
> likely to not have a /dev/null natively - so you should never need to
> O_CREAT /dev/null.  Furthermore, O_CREAT is wrong - /dev/null MUST be a
> char device, not a regular file, but if /dev/null is missing, you would
> be creating a regular file instead of a char device).

I will remove O_CREAT next time.

I appreciate your comment.

Seiji

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]