Re: [PATCH 17/36] attr: expose validity check for attribute names

[Stefan Beller <sbeller@google.com>]

On Sun, Oct 23, 2016 at 8:07 AM, Ramsay Jones
<ramsay@ramsayjones.plus.com> wrote:
>
>
> On 23/10/16 00:32, Stefan Beller wrote:
>> From: Junio C Hamano <gitster@pobox.com>
>>
>> Export attr_name_valid() function, and a helper function that
>> returns the message to be given when a given <name, len> pair
>> is not a good name for an attribute.
>>
>> We could later update the message to exactly spell out what the
>> rules for a good attribute name are, etc.
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Stefan Beller <sbeller@google.com>
>> ---
>
> [snip]
>
>> +extern int attr_name_valid(const char *name, size_t namelen);
>> +extern void invalid_attr_name_message(struct strbuf *, const char *, int);
>> +
>
> The symbol 'attr_name_valid()' is not used outside of attr.c, even
> by the end of this series. Do you expect this function to be used
> in any future series? (The export is deliberate and it certainly
> seems like it should be part of the public interface, but ...)
>
> In contrast, the 'invalid_attr_name_message()' function is called
> from code in pathspec.c, which relies on 'git_attr_counted()' to
> call 'attr_name_valid()' internally to check for validity. :-D

Yeah, I am taking over Junios patches and do not quite implement
what Junio thought I would. ;) So I guess it is a communication mismatch.

git_attr_counted is a wrapper around attr_name_valid in the way that
it either returns NULL when the attr name is invalid or it does extra work
and returns a pointer to an attr.

So I think for API completeness we'd want to keep attr_name_valid around,
as otherwise the API looks strange. But that doesn't seem like a compelling
reason, so I'll drop it from the header file and make it static.

Thanks,
Stefan