Re: [PATCH v5 05/14] xfs: Factor out new helper functions xfs_attr_rmtval_set

Allison Collins
2020-01-06T18:29:59Z

On 1/6/20 7:46 AM, Brian Foster wrote:
> On Wed, Dec 25, 2019 at 10:43:15AM -0700, Allison Collins wrote:
>>
>>
>> On 12/24/19 5:14 AM, Christoph Hellwig wrote:
>>> On Wed, Dec 11, 2019 at 09:15:04PM -0700, Allison Collins wrote:
>>>> Break xfs_attr_rmtval_set into two helper functions
>>>> xfs_attr_rmt_find_hole and xfs_attr_rmtval_set_value.
>>>> xfs_attr_rmtval_set rolls the transaction between the
>>>> helpers, but delayed operations cannot. We will use
>>>> the helpers later when constructing new delayed
>>>> attribute routines.
>>>
>>> Please use up the foll 72-ish characters for the changelog (also for
>>> various other patches).
>> Hmm, in one of my older reviews, we thought the standard line wrap length
>> was 68. Maybe when more folks get back from holiday break, we can have more
>> chime in here.
>>
>
> I thought it was 68 as well (I think that qualifies as 72-ish" at
> least), but the current commit logs still look short of that at a
> glance. ;P
>
> Brian
Ok I doubled checked, the last few lines do wrap a little early, but the
rest is correct for 68 because of the function names. We should
probably establish a number though. In perusing around some of the
other patches on the list, it looks to me like people are using 81?

Allison

>
>>>
>>> For the actual patch: can you keep the code in the order of the calling
>>> conventions, that is the lower level functions up and
>>> xfs_attr_rmtval_set at the bottom? Also please keep the functions
>>> static until callers show up (which nicely leads to the above order).
>>>
>>
>> Sure, will do.
>>
>> Allison
>>
>