On 18/01/17 23:05, Brandon Williams wrote: > On 01/18, Ramsay Jones wrote: >> >> Signed-off-by: Ramsay Jones <ramsay@xxxxxxxxxxxxxxxxxxxx> >> --- >> >> 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