GNU bug report logs - #54846
[PATCH] gnu: linux: Escape the values of string-type kconfig options

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 54846 in the body.
You can then email your comments to 54846 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: antlers <autumnalantlers <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: antlers <autumnalantlers <at> gmail.com>
Subject: [PATCH] gnu: linux: Escape the values of string-type kconfig options
Date: Sun, 10 Apr 2022 19:24:10 -0700

 * gnu/packages/linux.scm (config->string): add escape-string

Handles characters within the set
(char-set-intersection char-set:ascii char-set:printing), removing
those which are known to be unsupported.
---
 gnu/packages/linux.scm | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/gnu/packages/linux.scm b/gnu/packages/linux.scm
index b31fe0a580..60ae668fd9 100644
--- a/gnu/packages/linux.scm
+++ b/gnu/packages/linux.scm
@@ -761,6 +761,22 @@ (define %bpf-extra-linux-options
     ("CONFIG_IKHEADERS" . #t)))
 
 (define (config->string options)
+  (define (escape-string str)
+    "Returns STR with the escapes necessary to be read as a string-type
+    option's value. Handles characters within the set (char-set-intersection
+    char-set:ascii char-set:printing), removing those which are known to be
+    unsupported."
+    (fold (match-lambda* (((match? fmt) str)
+			  (transform-string str match?
+					    (cut format #f fmt <>))))
+	  str
+	  `((#\# "") ; No known way to escape # characters.
+	    (#\$ "$~a")
+	    ("\"\\'`" "\\~a")
+	    (";:()#" "\\\\~a")
+	    ("|" "\\\\\\~a")
+	    ;; No support for tabs, newlines, etc.
+	    (,(char-set->string (ucs-range->char-set 9 14)) ""))))
   (string-join (map (match-lambda
                       ((option . 'm)
                        (string-append option "=m"))
@@ -769,7 +785,9 @@ (define (config->string options)
                       ((option . #f)
                        (string-append option "=n"))
                       ((option . string)
-                       (string-append option "=\"" string "\"")))
+                       (string-append option "=\""
+				      (escape-string string)
+				      "\"")))
                     options)
                "\n"))
 
-- 
2.34.0

Message #8 received at 54846 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: antlers <autumnalantlers <at> gmail.com>
Cc: 54846 <at> debbugs.gnu.org
Subject: Re: bug#54846: [PATCH] gnu: linux: Escape the values of string-type
 kconfig options
Date: Tue, 12 Apr 2022 23:39:07 +0200

Hi,

antlers <autumnalantlers <at> gmail.com> skribis:

>  * gnu/packages/linux.scm (config->string): add escape-string
>
> Handles characters within the set
> (char-set-intersection char-set:ascii char-set:printing), removing
> those which are known to be unsupported.

[...]

>  (define (config->string options)
> +  (define (escape-string str)
> +    "Returns STR with the escapes necessary to be read as a string-type
> +    option's value. Handles characters within the set (char-set-intersection
> +    char-set:ascii char-set:printing), removing those which are known to be
> +    unsupported."

Nitpick: You can turn the docstring into a comment since the docstring
wouldn’t be accessible anyway.

> +    (fold (match-lambda* (((match? fmt) str)
> +			  (transform-string str match?
> +					    (cut format #f fmt <>))))

Please avoid tabs.

‘transform-string’ is from (texinfo string-utils), which is not imported
here.  IMO, we’d rather avoid depending on this module since it’s really
designed for the Texinfo machinery.

> +	  str
> +	  `((#\# "") ; No known way to escape # characters.
> +	    (#\$ "$~a")
> +	    ("\"\\'`" "\\~a")
> +	    (";:()#" "\\\\~a")
> +	    ("|" "\\\\\\~a")
> +	    ;; No support for tabs, newlines, etc.
> +	    (,(char-set->string (ucs-range->char-set 9 14)) ""))))

I wonder if this should be implemented in terms of ‘string-fold’
instead:

  (string-concatenate-reverse
    (string-fold (lambda (chr result)
                   (match chr
                     (#\# (cons "" result))
                     ;; …
                     (_ (cons (string chr) result))))
                 '()
                 str))

Thoughts?

Thanks,
Ludo’.

Message #13 received at 54846 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: antlers <autumnalantlers <at> gmail.com>
Cc: 54846 <at> debbugs.gnu.org
Subject: Re: bug#54846: [PATCH] gnu: linux: Escape the values of string-type
 kconfig options
Date: Thu, 28 Apr 2022 14:16:41 +0200

Hi antlers,

Did you have a chance to look into it?

TIA,
Ludo’.

Ludovic Courtès <ludo <at> gnu.org> skribis:

> Hi,
>
> antlers <autumnalantlers <at> gmail.com> skribis:
>
>>  * gnu/packages/linux.scm (config->string): add escape-string
>>
>> Handles characters within the set
>> (char-set-intersection char-set:ascii char-set:printing), removing
>> those which are known to be unsupported.
>
> [...]
>
>>  (define (config->string options)
>> +  (define (escape-string str)
>> +    "Returns STR with the escapes necessary to be read as a string-type
>> +    option's value. Handles characters within the set (char-set-intersection
>> +    char-set:ascii char-set:printing), removing those which are known to be
>> +    unsupported."
>
> Nitpick: You can turn the docstring into a comment since the docstring
> wouldn’t be accessible anyway.
>
>> +    (fold (match-lambda* (((match? fmt) str)
>> +			  (transform-string str match?
>> +					    (cut format #f fmt <>))))
>
> Please avoid tabs.
>
> ‘transform-string’ is from (texinfo string-utils), which is not imported
> here.  IMO, we’d rather avoid depending on this module since it’s really
> designed for the Texinfo machinery.
>
>> +	  str
>> +	  `((#\# "") ; No known way to escape # characters.
>> +	    (#\$ "$~a")
>> +	    ("\"\\'`" "\\~a")
>> +	    (";:()#" "\\\\~a")
>> +	    ("|" "\\\\\\~a")
>> +	    ;; No support for tabs, newlines, etc.
>> +	    (,(char-set->string (ucs-range->char-set 9 14)) ""))))
>
> I wonder if this should be implemented in terms of ‘string-fold’
> instead:
>
>   (string-concatenate-reverse
>     (string-fold (lambda (chr result)
>                    (match chr
>                      (#\# (cons "" result))
>                      ;; …
>                      (_ (cons (string chr) result))))
>                  '()
>                  str))
>
> Thoughts?
>
> Thanks,
> Ludo’.

Message #16 received at 54846 <at> debbugs.gnu.org (full text, mbox):

From: Antlers <autumnalantlers <at> gmail.com>
To: 54846 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: bug#54846: [PATCH] gnu: linux: Escape the values of string-type
 kconfig options
Date: Thu, 28 Apr 2022 20:18:04 +0000 (UTC)

Yeah, sorry for the silence, there's been a lot going on and being able to use strings in the first place is a comportable baseline of functionality- I don't feel that one should implement implicit escaping of a field until confident that all the corner cases are handled, and think that there are some subtle warts left. Haven't had that time iron those out, but I'll be glad to polish the details and can follow up within about a week once I've addressed my remaining cornerns about correctness. Thanks for bearing with me I fumble my way through the conventions of the mailing list and formatting, nitpicks are what I'm here for c:

I think transform-string is a gem for the task, the inputs are flexible and the author specifically cites better performance than string-fold in the (ice-9 texinfo) source, but I appreciate your point and can happily specialize it in-line.

----------------------------------------

Apr 28, 2022 5:16:46 AM Ludovic Courtès <ludo <at> gnu.org>:

> Hi antlers,
>
> Did you have a chance to look into it?
>
> TIA,
> Ludo’.
>
> Ludovic Courtès <ludo <at> gnu.org> skribis:
>
>> Hi,
>>
>> antlers <autumnalantlers <at> gmail.com> skribis:
>>
>>> * gnu/packages/linux.scm (config->string): add escape-string
>>>
>>> Handles characters within the set
>>> (char-set-intersection char-set:ascii char-set:printing), removing
>>> those which are known to be unsupported.
>>
>> [...]
>>
>>> (define (config->string options)
>>> +  (define (escape-string str)
>>> +    "Returns STR with the escapes necessary to be read as a string-type
>>> +    option's value. Handles characters within the set (char-set-intersection
>>> +    char-set:ascii char-set:printing), removing those which are known to be
>>> +    unsupported."
>>
>> Nitpick: You can turn the docstring into a comment since the docstring
>> wouldn’t be accessible anyway.
>>
>>> +    (fold (match-lambda* (((match? fmt) str)
>>> +             (transform-string str match?
>>> +                       (cut format #f fmt <>))))
>>
>> Please avoid tabs.
>>
>> ‘transform-string’ is from (texinfo string-utils), which is not imported
>> here.  IMO, we’d rather avoid depending on this module since it’s really
>> designed for the Texinfo machinery.
>>
>>> +     str
>>> +     `((#\# "") ; No known way to escape # characters.
>>> +       (#\$ "$~a")
>>> +       ("\"\\'`" "\\~a")
>>> +       (";:()#" "\\\\~a")
>>> +       ("|" "\\\\\\~a")
>>> +       ;; No support for tabs, newlines, etc.
>>> +       (,(char-set->string (ucs-range->char-set 9 14)) ""))))
>>
>> I wonder if this should be implemented in terms of ‘string-fold’
>> instead:
>>
>>   (string-concatenate-reverse
>>     (string-fold (lambda (chr result)
>>                    (match chr
>>                      (#\# (cons "" result))
>>                      ;; …
>>                      (_ (cons (string chr) result))))
>>                  '()
>>                  str))
>>
>> Thoughts?
>>
>> Thanks,
>> Ludo’.

Message #19 received at 54846 <at> debbugs.gnu.org (full text, mbox):

From: antlers <autumnalantlers <at> gmail.com>
To: 54846 <at> debbugs.gnu.org, Ludovic Courtès <ludo <at> gnu.org>
Subject: Re: bug#54846: [PATCH] gnu: linux: Escape the values of string-type
 kconfig options
Date: Sat, 7 May 2022 21:48:06 -0700

Hi! Still a busy week, working retail will do that, but this
investigation has been on the back burner long enough that it's become
clear; the kernel provides no reliable heuristic for determining the
appropriate escapes for a given option (least of all for out-of-tree
modules). Some options are explicitly expanded in Makefiles, others
aren't but become parts of filenames which are, and every special
character seems to create a syntax error on a whole new layer :c

I'm short on time to write anything up tonight, but have explored the
'behavior of', 'supported escapes within', and 'unsupported characters
of' CONFIG_SYSTEM_*_KEYS, CONFIG_CMDLINE, CONFIG_LOCALVERSION, and
CONFIG_DEFAULT_HOSTNAME (complete with associated errors for
unsupported chars within the set I initially referenced), and would be
glad to follow up with a brief summary purely for posterity; but
there's simply no elegant or complete approach unless this were to be
addressed by upstream(s). It's clearly peeved of me, and I'd take it
to those Makefiles, but for similar issues in out-of-tree modules that
I don't think I could address. It's been a lot of fun though.

I suppose we could factor out the escapes which are common to all
fields (which could be seen as eliminating /a/ layer, that of
Kconfig/conf.c itself), or address only specific common options (not a
serious suggestion!), but these each feel like inelegant solutions
which are more likely to introduce additional confusion when an option
doesn't behave correctly as transcribed out of a .config file, hence
my all-or-nothing mindset.

While I'm here, I once wrote a bash script which would set options via
the kernel's 'config' utility (not worth using over the existing
method of appending to .config, doesn't do any validation), and was
having issues with some configurations because not every option I
tried to set had it's dependency clauses satisfied. This isn't an
issue when using menuconfig because options don't even appear until
their dependencies are satisfied, but scripts can't tell whether the
options they're setting are visible. At the time I just had it
double-check after a run of `make oldconfig` (when validation is
actually done) and emit warnings. I'm not confident that it caught
every issue by just using the 'config' utility or grepping .config,
but it was good enough.

Now, I've got this crazy idea to compile conf.c as a shared library
and link against it at runtime via Guile's FFI, in order to a) learn
how that works(!), and b) use the kernels own utilities to reconstruct
the options-graph (a 'menu' object) and emit correct warnings when
setting options that aren't visible, or even actively ensure
dependencies are satisfied. In brief, would this be Guix-y?

Message #24 received at 54846-done <at> debbugs.gnu.org (full text, mbox):

From: Antlers <autumnalantlers <at> gmail.com>
To: 54846-done <at> debbugs.gnu.org
Subject: Re: bug#54846: [PATCH] gnu: linux: Escape the values of string-type
 kconfig options
Date: Tue, 17 May 2022 15:54:00 +0000 (UTC)

[won't-fix]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.