On Wed, 05 Apr 2017 15:07:40 -0400 "Ted Unangst" <[email protected]> wrote:
> [email protected] wrote: > > No functional or user-visible changes here. > > On that note, where did the "1024" (1023-char) come from? Is there > > anyone who has environment variables whose name goes near 1023 > > chars? > > Probably not, but there's not much benefit to be artifically limiting > here. I guess in this case it's better than strdup'ing a super short-lived variable like this. I was thinking in the lines of having such a large variable on the stack where the space is unused. > > > @@ -95,7 +97,7 @@ createenv(struct rule *rule) > > if ((eq = strchr(e, '=')) == NULL || eq == > > e) continue; > > len = eq - e; > > - if (len > sizeof(name) - 1) > > + if (len > VARNAME_MAX) > > continue; > > memcpy(name, e, len); > > name[len] = '\0'; > > I don't like changes like this because if the size ever changes > again, it's possible for the check to become decoupled. > I see where you are coming from here. Would it be worth just keeping the "#define" change? The 1024 limit is in 2 places and I can see a situation where they end up out of sync. Also I was thinking about this: if (len > (sizeof(name) - 1)) I guess the C order of operations will not require the inner parentheses, but the intent seems clearer this way without having to get caught up with the specifics of C.
