Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1510253 > unrolled thread

Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 16:20 +0200
    Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Frank Rowand <frowand.list@gmail.com> - 2016-10-27 18:40 +0200
      Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 19:00 +0200
        Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Frank Rowand <frowand.list@gmail.com> - 2016-10-27 20:30 +0200
          Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from  local function names Rob Herring <robh+dt@kernel.org> - 2016-10-27 22:30 +0200

#1510253 — Re: [RFC PATCH 06/13] of: Remove prefix "__of_" and prefix "__" from local function names

On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
> From: Frank Rowand <frank.rowand@am.sony.com>

I prefer to leave the prefixes and this is getting into pointless churn.

>
> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
> ---
>  drivers/of/resolver.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)

[toc] | [next] | [standalone]

#1510468

On 10/27/16 05:47, Rob Herring wrote:
> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
> 
> I prefer to leave the prefixes and this is getting into pointless churn.
> 
>>
>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>> ---
>>  drivers/of/resolver.c | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

If I was just submitting this as a single patch, I would agree.

But since I am making so many other changes, I think it makes
sense to do this as part of this series.  It is broken apart
as a separate patch to be easy to review and not pollute any
of the other patches in the series.

The prefixes add no value for a local function, but they do
add noise when reading code.

The changes are local to this file and do not impact anything
else.

Looking at the single patch, it does seem like churn.  But
looking at the entire file before the set of changes and
after the set of changes, I find the file much easier to
read afterwards.  Each individual patch may make a small
contribution to the end result, but the combination of all
of them is significant.

[toc] | [prev] | [next] | [standalone]

#1510490

On Thu, Oct 27, 2016 at 11:35 AM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 10/27/16 05:47, Rob Herring wrote:
>> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> I prefer to leave the prefixes and this is getting into pointless churn.
>>
>>>
>>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>>> ---
>>>  drivers/of/resolver.c | 10 +++++-----
>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>
> If I was just submitting this as a single patch, I would agree.
>
> But since I am making so many other changes, I think it makes
> sense to do this as part of this series.  It is broken apart
> as a separate patch to be easy to review and not pollute any
> of the other patches in the series.
>
> The prefixes add no value for a local function, but they do
> add noise when reading code.

The value is when reading the calling function, you know the function
is a DT related function. You don't know it's a static function
without looking up the function name. That said, I wouldn't object to
code originally written either way, I just don't see the value in
changing it.

Rob

[toc] | [prev] | [next] | [standalone]

#1510563

On 10/27/16 09:58, Rob Herring wrote:
> On Thu, Oct 27, 2016 at 11:35 AM, Frank Rowand <frowand.list@gmail.com> wrote:
>> On 10/27/16 05:47, Rob Herring wrote:
>>> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>>>> From: Frank Rowand <frank.rowand@am.sony.com>
>>>
>>> I prefer to leave the prefixes and this is getting into pointless churn.
>>>
>>>>
>>>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>>>> ---
>>>>  drivers/of/resolver.c | 10 +++++-----
>>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>
>>
>> If I was just submitting this as a single patch, I would agree.
>>
>> But since I am making so many other changes, I think it makes
>> sense to do this as part of this series.  It is broken apart
>> as a separate patch to be easy to review and not pollute any
>> of the other patches in the series.
>>
>> The prefixes add no value for a local function, but they do
>> add noise when reading code.
> 
> The value is when reading the calling function, you know the function
> is a DT related function. You don't know it's a static function

It is more than that.  A common convention in drivers/of/ is that
function blah() acquires a lock, calls function __blah(), and
releases the lock.  Any function other than blah() that wants
to call __blah() must also hold the proper lock.  The functions
whose name this patch changes do not fit this pattern.


> without looking up the function name. That said, I wouldn't object to
> code originally written either way, I just don't see the value in
> changing it.
> 
> Rob
> 

[toc] | [prev] | [next] | [standalone]

#1510629

On Thu, Oct 27, 2016 at 1:25 PM, Frank Rowand <frowand.list@gmail.com> wrote:
> On 10/27/16 09:58, Rob Herring wrote:
>> On Thu, Oct 27, 2016 at 11:35 AM, Frank Rowand <frowand.list@gmail.com> wrote:
>>> On 10/27/16 05:47, Rob Herring wrote:
>>>> On Tue, Oct 25, 2016 at 3:58 PM,  <frowand.list@gmail.com> wrote:
>>>>> From: Frank Rowand <frank.rowand@am.sony.com>
>>>>
>>>> I prefer to leave the prefixes and this is getting into pointless churn.
>>>>
>>>>>
>>>>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>>>>> ---
>>>>>  drivers/of/resolver.c | 10 +++++-----
>>>>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>>>
>>>
>>> If I was just submitting this as a single patch, I would agree.
>>>
>>> But since I am making so many other changes, I think it makes
>>> sense to do this as part of this series.  It is broken apart
>>> as a separate patch to be easy to review and not pollute any
>>> of the other patches in the series.
>>>
>>> The prefixes add no value for a local function, but they do
>>> add noise when reading code.
>>
>> The value is when reading the calling function, you know the function
>> is a DT related function. You don't know it's a static function
>
> It is more than that.  A common convention in drivers/of/ is that
> function blah() acquires a lock, calls function __blah(), and
> releases the lock.  Any function other than blah() that wants
> to call __blah() must also hold the proper lock.  The functions
> whose name this patch changes do not fit this pattern.

Okay, fair enough.

Rob

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web