linux-kernel - Re: [PATCH v4 03/31] kconfig: reference environment variables directly and remove 'option env='

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAFkk2KQZV35nQFALHz4ADrrBXOaLirVEf1Z5TAU3zuHDPL1SWw@mail.gmail.com>
Date:   Sat, 26 May 2018 22:47:49 +0200
From:   Ulf Magnusson <ulfalizer@...il.com>
To:     Masahiro Yamada <yamada.masahiro@...ionext.com>
Cc:     Linux Kbuild mailing list <linux-kbuild@...r.kernel.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Sam Ravnborg <sam@...nborg.org>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Nicholas Piggin <npiggin@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Emese Revfy <re.emese@...il.com>, X86 ML <x86@...nel.org>
Subject: Re: [PATCH v4 03/31] kconfig: reference environment variables
 directly and remove 'option env='

On Thu, May 24, 2018 at 6:45 AM, Masahiro Yamada
<yamada.masahiro@...ionext.com> wrote:
> 2018-05-21 20:06 GMT+09:00 Ulf Magnusson <ulfalizer@...il.com>:
>
>>>
>>> static char *__expand_string(const char **str, bool (*is_end)(const char *))
>>> {
>>>         const char *in, *prev_in, *p;
>>>         char *new, *out;
>>>         size_t outlen;
>>>
>>>         out = xmalloc(1);
>>>         *out = 0;
>>>
>>>         p = in = *str;
>>>
>>>         while (1) {
>>>                 if (*p == '$') {
>>>                         prev_in = in;
>>>                         in = p + 1;
>>>                         new = expand_dollar(&in);
>>>                         outlen = strlen(out) + (p - prev_in) + strlen(new) + 1;
>>>                         out = xrealloc(out, outlen);
>>>                         strncat(out, prev_in, p - prev_in);
>>>                         strcat(out, new);
>>>                         free(new);
>>>                         p = in;
>>>                         continue;
>>>                 }
>>>
>>>                 if (is_end(p))
>>>                         break;
>>>
>>>                 p++;
>>>         }
>>>
>>>         outlen = strlen(out) + (p - in) + 1;
>>>         out = xrealloc(out, outlen);
>>>         strncat(out, in, p - in);
>>>
>>>         *str = p;
>>>
>>>         return out;
>>> }
>>>
>>> static bool is_end_of_str(const char *s)
>>> {
>>>         return !*s;
>>> }
>>>
>>> char *expand_string(const char *in)
>>> {
>>>         return __expand_string(&in, is_end_of_str);
>>> }
>>>
>>> static bool is_end_of_token(const char *s)
>>> {
>>>         return !(isalnum(*s) || *s == '_' || *s == '-' || *s == '.' ||
>>> *s == '/');
>>> }
>>>
>>> char *expand_one_token(const char **str)
>>> {
>>>         return __expand_string(str, is_end_of_token);
>>> }
>>
>> Yep, something like that would be nicer I think.
>>
>> This variant might work too (untested):
>>
>>   dollar_i = p;
>>   p++;
>>   expansion = expand_dollar(&p);
>>
>>   out = xrealloc(out, strlen(out) + (dollar_i - in)
>>                       + strlen(expansion) + 1);
>>   strncat(out, in, dollar_i - in);
>>   strcat(out, expansion);
>>   free(expansion);
>>
>>   in = p;
>>
>>   continue;
>>
>> The p++ would disappear if expand_dollar() took a pointer to the '$'.
>>
>
>
> I took the variable name "expansion" because
> it is more descriptive than "new".
>
>
> I rewrote like follows:
>
>
> static char *__expand_string(const char **str, bool (*is_end)(const char *))
> {
>         const char *in, *p;
>         char *expansion, *out;
>         size_t in_len, out_len;
>
>         out = xmalloc(1);
>         *out = 0;
>         out_len = 1;
>
>         p = in = *str;
>
>         while (1) {
>                 if (*p == '$') {
>                         in_len = p - in;
>                         p++;
>                         expansion = expand_dollar(&p);
>                         out_len += in_len + strlen(expansion);
>                         out = xrealloc(out, out_len);
>                         strncat(out, in, in_len);
>                         strcat(out, expansion);
>                         free(expansion);
>                         in = p;
>                         continue;
>                 }
>
>                 if (is_end(p))
>                         break;
>
>                 p++;
>         }
>
>         in_len = p - in;
>         out_len += in_len;
>         out = xrealloc(out, out_len);
>         strncat(out, in, in_len);
>
>         /* Advance 'str' to the end character */
>         *str = p;
>
>         return out;
> }
>
>
>
> I used "out_len" to remember the length of "out"
> instead of calculating strlen(out) every time.
>
> I do not need dollar_p.
>
>
>
>
> --
> Best Regards
> Masahiro Yamada

Looks good to me.

Could keep some 'out' pointer to avoid the str(n)cat()s too, but
pretty sure it's overkilling it. Should have some general string
buffer helpers at that point I think. :)

Cheers,
Ulf