On Nov 30, 2015, at 11:26 AM, Kevin Wolf wrote:
> 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.
I just think that using "Error" does communicate the fact that something is
wrong
a lot better than just printing the message.
