On 10/11, Stefan Beller wrote:
> On Tue, Oct 11, 2016 at 10:28 AM, Brandon Williams <bmwill@xxxxxxxxxx> wrote:
> > On 10/10, Stefan Beller wrote:
> >> From: Junio C Hamano <gitster@xxxxxxxxx>
> >> -static int invalid_attr_name(const char *name, int namelen)
> >> +int attr_name_valid(const char *name, size_t namelen)
> >> {
> >> /*
> >> * Attribute name cannot begin with '-' and must consist of
> >> * characters from [-A-Za-z0-9_.].
> >> */
> >> if (namelen <= 0 || *name == '-')
> >> - return -1;
> >> + return 0;
> >> while (namelen--) {
> >> char ch = *name++;
> >> if (! (ch == '-' || ch == '.' || ch == '_' ||
> >> ('0' <= ch && ch <= '9') ||
> >> ('a' <= ch && ch <= 'z') ||
> >> ('A' <= ch && ch <= 'Z')) )
> >> - return -1;
> >> + return 0;
> >> }
> >> - return 0;
> >> + return -1;
> >> +}
> >
> > Whats the reason behind returning -1 for a valid attr name vs 1?
> >
> At first I thought the two different return path for -1 are different
> error classes (one being just a minor error compared to the other),
> but they are not, so having the same code there makes sense.
>
> So I think we could change it to +1 in this instance, as a non zero
> value would just indicate the attr name is not valid, but not necessarily
> an error.
Wouldn't a +1 indicate that the attr name is valid while the 0
indicates that it is invalid? It looks to me like we are checking each
character and if we run into one that doesn't work then we have an error
and return 0 indicating that the attr name we are checking is invalid,
otherwise the name is valid and we would want to return a +1 indicating
a success.
Am I understanding the intent of this function properly?
--
Brandon Williams