ltp.lists.linux.it archive mirror
 help / color / mirror / Atom feed
From: Richard Palethorpe <rpalethorpe@suse.de>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH] lib: tst_bool_expr: Add support for strings
Date: Wed, 11 Nov 2020 17:38:52 +0000	[thread overview]
Message-ID: <87k0urpzdf.fsf@suse.de> (raw)
In-Reply-To: <20201111143744.GA16874@yuki.lan>

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> Why not skip the whole switch statement if in_string and just check for
>> the closing '"' instead?
>
> I guess that we can instead do:
>
> diff --git a/lib/tst_bool_expr.c b/lib/tst_bool_expr.c
> index dd147cde3..387c38b91 100644
> --- a/lib/tst_bool_expr.c
> +++ b/lib/tst_bool_expr.c
> @@ -81,6 +81,13 @@ static unsigned int tokenize(const char *expr, struct tst_expr_tok *last)
>                         token_cnt += new_tok(&last, &expr[j], i - j);
>                         j = i+1;
>                 break;
> +               case '"':
> +                       while (expr[i+1] != '"' && expr[i+1])
> +                               i++;
> +
> +                       if (expr[i+1] == '"')
> +                               i++;
> +               break;
>                 default:
>                 break;
>                 }
>
>
> Not sure if this is more readable.

It's probably easier to understand, it atleast keeps string processing
separate.

>
>> >  		break;
>> >  		default:
>> >  		break;
>> > -- 
>> > 2.26.2
>> 
>> It should probably be an error if tokenize exits with in_string=1?
>
> Well my opinion is that we do not care since we will get error later on, in
> case of the kernel config we would get obviously wrong variable value. But if
> you think that this is important we can change the return type to int
> and return -1 on tokenizer failure.

I suppose it depends how much later and how such an error would look. If
the error will happen directly after leaving this function then I don't
think it matters either, but if it is possible that it manages to
complete the whole evaluation process before failing with TCONF because
the vars don't match then this has the potential to waste some time.

-- 
Thank you,
Richard.

  reply	other threads:[~2020-11-11 17:38 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-11-11 13:11 [LTP] [PATCH] lib: tst_bool_expr: Add support for strings Cyril Hrubis
2020-11-11 14:19 ` Richard Palethorpe
2020-11-11 14:37   ` Cyril Hrubis
2020-11-11 17:38     ` Richard Palethorpe [this message]
2020-11-12 13:02       ` Cyril Hrubis
2020-11-12 14:02         ` Richard Palethorpe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k0urpzdf.fsf@suse.de \
    --to=rpalethorpe@suse.de \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).