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

On Sun, Oct 23, 2016 at 8:07 AM, Ramsay Jones
[off-list ref] wrote:

On 23/10/16 00:32, Stefan Beller wrote:

quoted

From: Junio C Hamano <redacted>

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 <redacted>
Signed-off-by: Stefan Beller <redacted>
---

[snip]

quoted

+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