On 05/04/2010 04:58 PM, Eric Blake wrote:
> On 04/30/2010 09:44 AM, Cole Robinson wrote:
>> qemuReadLogOutput early VM death detection is racy and won't always work.
>> Startup then errors when connecting to the VM monitor. This won't report
>> the emulator cmdline output which is typically the most useful diagnostic.
>>
>> Check if the VM has died at the very end of the monitor connection step,
>> and if so, report the cmdline output.
>
>> +static void
>> +qemuReadLogFD(int logfd, char *buf, int maxlen, int off)
>> +{
>> + int ret;
>> + char *tmpbuf = buf+off;
>
> Isn't the style to separate operators by space? 'buf + off'
>
>> +
>> + ret = saferead(logfd, tmpbuf, maxlen-off-1);
>
> Likewise, 'maxlen - off - 1'.
>
>> + if (ret < 0) {
>> + ret = 0;
>> + }
>> +
>> + *(tmpbuf+ret) = '\0';
>
> Stylistically, I like 'tmpbuf[ret]' better than '*(tmpbuf+ret)'.
>
Doh, I was overthinking here :)
>> +}
>> +
>> static int
>> qemudWaitForMonitor(struct qemud_driver* driver,
>> virDomainObjPtr vm, off_t pos)
>> {
>> - char buf[4096]; /* Plenty of space to get startup greeting */
>> + char buf[4096] = "\0"; /* Plenty of space to get startup greeting */
>
> "" is sufficient; you don't have to use "\0" to zero-initialize a
> statically sized char[].
>
> But overall, the patch looks sane; I didn't see any logic errors.
> ACK with the style nits addressed.
>
Thanks for the review, I made these style adjustments and pushed.
- Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list