[Mesa-dev] [PATCH 08/12] glcpp: Use strpbrk in the line continuations pass

[Vladislav Egorov]

Oh. Thanks! This is embarrassing. I have no idea how did I manage to 
miss it. Line continuations are so rare that only two games in my 
shader-db use them -- Talos Principle and Serious Sam (both use Serious 
Engine). And neither of them hit this condition. I probably need to fuzz 
this thing or something.

I'll send fixup.

07.01.2017 22:53, Gustaw Smolarczyk пишет:
> 7 sty 2017 20:04 "Vladislav Egorov" <vegorov180 at gmail.com 
> <mailto:vegorov180 at gmail.com>> napisał(a):
>
>     To find newlines the line continuations removal pass uses strchr()
>     twice -- first time to find '\n', second time to find '\r'. Now,
>     if the shader uses Unix line separators '\n', the file could contain
>     no '\r' at all, so each time it will scan to the end of the shader.
>     Use strpbrk() standard function instead that was specifically created
>     to search many characters at once.
>     ---
>      src/compiler/glsl/glcpp/pp.c | 28 ++++++++--------------------
>      1 file changed, 8 insertions(+), 20 deletions(-)
>
>     diff --git a/src/compiler/glsl/glcpp/pp.c
>     b/src/compiler/glsl/glcpp/pp.c
>     index c196868..c4c0196 100644
>     --- a/src/compiler/glsl/glcpp/pp.c
>     +++ b/src/compiler/glsl/glcpp/pp.c
>     @@ -231,7 +231,6 @@ remove_line_continuations(glcpp_parser_t *ctx,
>     const char *shader)
>      {
>             struct string_buffer sb;
>             const char *backslash, *newline, *search_start;
>     -        const char *cr, *lf;
>              char newline_separator[3];
>             int collapsed_newlines = 0;
>             int separator_len;
>     @@ -266,23 +265,19 @@ remove_line_continuations(glcpp_parser_t
>     *ctx, const char *shader)
>              * examining the first encountered newline terminator, and
>     using the
>              * same terminator for any newlines we insert.
>              */
>     -       cr = strchr(search_start, '\r');
>     -       lf = strchr(search_start, '\n');
>     +       newline = strpbrk(search_start, "\r\n");
>
>             newline_separator[0] = '\n';
>             newline_separator[1] = '\0';
>             newline_separator[2] = '\0';
>
>     -       if (cr == NULL) {
>     +       if (newline == NULL) {
>                     /* Nothing to do. */
>     -       } else if (lf == NULL) {
>     -               newline_separator[0] = '\r';
>     -       } else if (lf == cr + 1) {
>     -               newline_separator[0] = '\r';
>     -               newline_separator[1] = '\n';
>     -       } else if (cr == lf + 1) {
>     -               newline_separator[0] = '\n';
>     -               newline_separator[1] = '\r';
>     +       } else if (newline[1] == '\n' || newline[1] == '\r') {
>
> What if we have \n\n ? Won't it match it as a single newline?
>
> Regards,
> Gustaw

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170108/2fcd6a94/attachment.html>