Re: [Qemu-devel] [PATCH v9] block/raw-posix.c: Make physical devices usable in QEMU under Mac OS X host

[Kevin Wolf <kwolf@redhat.com>]

[-- Attachment #1: Type: text/plain, Size: 2260 bytes --]

Am 30.11.2015 um 17:19 hat Eric Blake geschrieben:
> On 11/27/2015 12:35 PM, Programmingkid wrote:
> 
> >> Unusual indentation; more typical is:
> >>
> >> | static kern_return_t FindEjectableOpticalMedia(io_iterator_t
> >> *mediaIterator,
> >> | char *mediatType)
> > 
> > I agree. I wanted the second long to be right justified with the 80 character line count.
> 
> No.  We don't right-justify code to 80 columns.  That's not how it is
> done.  Trying to do it just makes you look like the proverbial 'kid' in
> your pseudonym, rather than an adult to be taken seriously.
> 
> Really, PLEASE follow the indentation patterns of the rest of the code
> base - where continued lines are left-justified to be underneath the
> character after (, and NOT right-justified to 80 columns.  Violating
> style doesn't make your code invalid, but does make your patches less
> likely to be applied.
> 
> 
> >>> +        /* If you found a match, leave the loop */
> >>> +        if (*mediaIterator != 0) {
> >>> +            DPRINTF("Matching using %s\n", matching_array[index]);
> >>> +            snprintf(mediaType, strlen(matching_array[index])+1, "%s",
> >>
> >> Spaces around binary '+'.
> > 
> > What's wrong with no spaces around the plus sign?
> 
> Again, the prevailing conventions in the code base is that you put
> spaces around every binary operator.  Yes, there is existing old code
> that does not meet the conventions, but it is not an excuse to add new
> code that is gratuitously different.
> 
> > 
> >>
> >>> +    /* if a working partition on the device was not found */
> >>> +    if (partition_found == false) {
> >>> +        error_setg(errp, "Error: Failed to find a working partition on "
> >>> +                                                                     "disc!\n");
> >>
> >> and I already pointed out on v8 that this is not the correct usage of
> >> error_setg().  So, here's hoping v10 addresses the comments here and
> >> elsewhere.
> > 
> > Kevin Wolf wanted it this way. What would you do instead?
> 
> Keven and I both want you to use error_setg(), but to use it correctly -
> and the correct way is to NOT supply a trailing \n.

Nor leading "Error:", for that matter.

Kevin

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]