[bug#54352] [PATCH] services: dnsmasq: Add more options.

[Remco van 't Veer]

2022/03/21 19:36, Maxime Devos:

> [[PGP Signed Part:Undecided]]
> Ludovic Courtès schreef op ma 21-03-2022 om 16:22 [+0100]:
>> I think this suggestion is beyond the scope of this review: we’ve never
>> used sanitizers like this before (or almost), and this particular piece
>> of code doesn’t use them.
>>
>> Also, with the recent discussion about the introduction of contracts,
>> I’d rather wait an use contracts everywhere once they’re available.
>
> Seems reasonable to me, given that the specifics weren't discussed yet,
> although _everywhere_ (for all procedures, records, ...) seems a bit
> much, unless you meant every field of the dnsmasq record.

I can add something like the following:

  (define (assert-boolean value)
    (unless (false-if-exception (boolean? value))
      (error-out (format #f "expected a boolean, got: ~s" value)))
    value)

and use it to do

  (sanitize assert-boolean)

on all boolean fields of dnsmasq but I agree with Ludo about this being
a bigger issue which can be solved much more elegantly (including i18n
and source location etc).  In the above I borrowed error-out from knot
in the same file which also doesn't do i18n and source locations.

I'd like to leave it out because the codebase has plenty more unverified
booleans and basic if-statements which go the unexpected route when
passed a "false" string.