Re: [PATCH 1/2] btrfs-progs: print-tree: Use macro to replace immediate number for readable flags string buffer length

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

 




On 2018年03月22日 21:39, David Sterba wrote:
> On Thu, Mar 22, 2018 at 10:40:22AM +0200, Nikolay Borisov wrote:
>> On 22.03.2018 10:32, Qu Wenruo wrote:
>>> On 2018年03月22日 16:20, Nikolay Borisov wrote:
>>>> On 22.03.2018 09:37, Qu Wenruo wrote:
>>>>> In print-tree, we have a lot of parsers to convert numeric flags to
>>>>> human readable string.
>>>>>
>>>>> For the buffer size we're using immediate numbers for all their callers.
>>>>> Change this to macro so it will be much easier for us to expand the
>>>>> buffer size.
>>>>>
>>>>> Signed-off-by: Qu Wenruo <wqu@xxxxxxxx>
>>>>
>>>> The changes look good per se, however I'd like to have all such macros
>>>> grouped at the beginning of the file - in their own section so to speak.
>>>>
>>>
>>> That's also what I considered.
>>> However this makes stack consumption larger for some callers who don't
>>> really need that much buffer size.
>>
>> I just meant to have the multiple defines you've introduced grouped at
>> the beginning of the file. Not having one define for everyone. This is
>> done purely for aesthetics purposes, though I'm not too hung up on that
>> so if you decide to ignore it, it's fine.
> 
> No, that's actually what I prefer, the first part of header or source is
> for defintions. Unless it's really local to the function or single use.

All right, I'll change the layout.

But leave some comment for each *_flags_to_str() function to info later
contributor what to change.

Thanks,
Qu

> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Attachment: signature.asc
Description: OpenPGP digital signature


[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux