Re: [PATCH] attr: mark a file-local symbol as static

On 18/01/17 23:05, Brandon Williams wrote:

On 01/18, Ramsay Jones wrote:

quoted

Signed-off-by: Ramsay Jones <redacted>
---

Hi Brandon,

If you need to re-roll your 'bw/attr' branch, could you please
squash this into the relevant patch (commit 8908457159,
"attr: use hashmap for attribute dictionary", 12-01-2017).

Also, I note that, although they are declared as part of the
public attr api, attr_check_clear() and attr_check_reset() are
also not called outside of attr.c. Are these functions part of
the public api?

Also, a minor point, but attr_check_reset() is called (line 1050)
before it's definition (line 1114). This is not a problem, given
the declaration in attr.h, but I prefer definitions to come before
use, where possible.

Thanks!

ATB,
Ramsay Jones

Yes of course, I believe Stefan also pointed that out earlier today so I
have it fixed locally.

Yep, I noticed Stefan's email just a few minutes after I hit
send! ;-)

For attr_check_clear() and attr_check_reset() the intent is that they
are the accepted way to either clear or reset the attr_check structure.
Currently most users of the attribute system don't have a need to clear
or reset the structure but there could be future callers who need that
functionality.  If you feel like they shouldn't be part of the api right
now then I'm open to changing that for this series.

No, I just wanted to check that they were intended to be part of
the public api and that you anticipate additional callers in the
future.

[I would still prefer definitions before use, but many people would
not agree with me, so ...]

ATB,
Ramsay Jones