GNU bug report logs - #56046
[PATCH] services: mail: add opensmtpd records to enhance opensmtpd-configuration.

Previous Next

Full log

View this message in rfc822 format

From: jbranso <at> dismail.de
To: "Liliana Marie Prikler" <liliana.prikler <at> gmail.com>
Cc: Joshua Branson <joshua <at> gnucode.me>, 56046 <at> debbugs.gnu.org
Subject: [bug#56046] [PATCH opensmtpd-records v3] services (opensmtpd): add opensmtpd records to enhance opensmtpd-configuration.
Date: Wed, 28 Dec 2022 20:42:37 +0000

December 28, 2022 3:04 PM, "Liliana Marie Prikler" <liliana.prikler <at> gmail.com> wrote:

> Am Dienstag, dem 27.12.2022 um 19:16 -0500 schrieb Joshua Branson:
> 
>> Are you giving me a triple A+ ? :) Org generated the the like that. I
>> think you mentioned that I should use fractions last time. Sorry I
>> did not do that.
> 
> Do you have to convert your documentation from org? Writing Texinfo
> code manually is an option, as is generating it from define-
> configuration IIRC. There is also nothing wrong with manually touching
> up generated docs, but I imagine doing so consistently might be a bit
> more adventurous.

I wrote the first draft of the documentation in org.  Then converted it
to texinfo.  I have been writing in texinfo ever since.  :)

> 
>> If I wait 'til I implement every one of your suggestions, I will
>> probably never submit it. I am really probably "perfecting" this
>> service.
> 
> You can submit whatever, but don't expect me or any other committer to
> upstream the patches while there are open points to address.

Of course.  :)  I wish I could implement all of your suggestions sooner, 
but I am still learning.  And I might be a bit of a slow coder.  :(

>> Instead of a string, take 'no-verify as symbol perhaps?
>> 
>> Sounds good to me. May I ask why you prefer a symbol instead of a
>> string?
> 
> Symbols can be compared with eq?, case et al.
> 
>>> -;;;
>>> ;;; OpenSMTPD.
>>> ;;;
>>> +;;; This next bit of code helps me create my own sanitizer
>>> functions.
>>> +
>>> +;; some fieldnames have a default value of #f, which is ok. 
>>> They
>>> cannot have
>>> +;; a value of #t.
>>> +;; for example opensmtpd-table-data can be #f, BUT NOT true.
>>> +;; my/sanitize procedure tests values to see if they are of the
>>> right kind.
>>> +;; procedure false? is needed to allow fields like 'values' to
>>> be
>>> blank,
>>> +;; (empty), or #f BUT also have a value like a list of strings.
>> Use less egocentric comments ;)
>> 
>> I'm not sure what you mean here? I know I had a comment in my task
>> list that said something like my sanitizer function are probably
>> better than those found in guix. Apologies for that.
> 
> For what it's worth, it definitely wasn't I. [1]
> 
>>> +(define (false? var)
>>> +  (eq? #f var))
>>> +
>>> +;; TODO I have to have this procedure, or I need to change
>>> my/sanitize
>>> +;; procedure.
>>> +(define (my-file-exists? file)
>>> +  (and (string? file)
>>> +       (access? file F_OK)))
>> Does file-exists? not work for you?
>> 
>> The file-exists? function causes my-sanitize function to break.
> 
> Why?
> 
>> I think.
> 
> Prove it.

Oh, the last time I used guile's file-exist? It broke a one of 
my tests.  I intend to use guile's file-exist? I just have not
figured out how to yet.  :)

> 
>> If you get rid of it, then what happens when a user types in (file
>> 4), you get an raise-exception.
> 
> (file-exists? "(file 4)") ; => #f

The way I am sanitizing it, something like this happens.  
A silly user puts this in their configuration:

(opensmtpd-configuration
   (config-file 4))


(file-exists? (openstmtpd-configuration-config-file record))

(file-exists? 4)  => raise-exception

>> I can probably just rework my-sanitizer function to
>> deal with that possibility, but I have not yet. I would love some
>> guidance on how to do that. Because I feel like having to handle that
>> exception is hard.
> 
> From the Guile manual:
> 
> -- Scheme Procedure: stat object [exception-on-error?]
> -- C Function: scm_stat (object, exception_on_error)
> [...]
> If the optional EXCEPTION_ON_ERROR argument is true, which is the
> default, an exception will be raised if the underlying system call
> returns an error, for example if the file is not found or is not
> readable. Otherwise, an error will cause ‘stat’ to return ‘#f’.
> 
> Now, in (ice-9 boot-9), file-exists? is defined (assuming posix) as
> 
> (lambda (str)
> (->bool (stat str #f)))
> 
> Thus, I am pretty sure that no exception should be raised from the
> check ;)
> 
>>> +;; This procedure takes in a var and a list of procedures.  It
>>> loops
>>> through
>>> +;; list of procedures passing in var to each.
>>> +;; if one procedure returns #t, the function returns true. 
>>> Otherwise #f.
>>> +;; TODO for fun rewrite this using map
>>> +;; If I rewrote it in map, then it may help with sanitizing.
>>> +;; eg: I could then potentially easily sanitize vars with lambda
>>> procedures.
>>> +(define (is-value-right-type? var list-of-procedures record
>>> fieldname)
>>> +  (if (null? list-of-procedures)
>>> +      #f
>>> +      (if ((car list-of-procedures) var)
>>> +          #t
>>> +          (is-value-right-type? var (cdr list-of-procedures)
>>> record
>>> +                                fieldname))))
>> Alternatively, (any (cut <> var) list-of-procedures).
>> 
>> You mentioned that in the last review, I just can't figure out how to
>> use your
>> suggestion. This is the code that I have in the task list WIP:
>> 
>> *** TODO simplify my sanitizing funcions  (any (cut <> var))
>> 
>> #+BEGIN_SRC scheme
>> (use-modules (ice-9 curried-definitions)
>> (srfi srfi-26))
>> 
>> (define (((expect-any predicates) record field) var)
>> (if (any (cut <> var) predicates)
>> var
>> (begin
>> ;; code code code
>> ;; how do I tell the user which function failed?
>> (display "error")
>> (throw 'bad! var))))
> 
> All of them failed, that's the point. As for constructing a string
> from a list of procedures, see list-of-procedures->string.
> 
>> ;; here is how you use it.
>> (name opensmtpd-table-name ;; string
>> (default #f)
>> (sanitize (lambda (var)
>> (((expect-any (list string? number?)) "hello"
>> "that") var))))
>> 
>> #+END_SRC
>> 
>> Does that look close to what you want? I feel like it is way off, but
>> I don't know. Honestly when I say this suggestion I was completely
>> blown away, I have been using (any ) and (every) in a few places to
>> get rid of some uses of primitive eval.
> 
> I don't see that, but I do see functions that have been dropped still
> mentioned in the ChangeLog. Another hint at this patch being too
> convoluted for its own sake ;)

How would I unconvoluted it?  Use define-configuration?  

> 
>> This procedure needs a proper name, like sanitize/check-type, but
>> more importantly, why not simply use define-configuration?
>> 
>> Yes! I have slowly been realizing that I have been clumsily re-
>> inventing define-configuration. I hope to switch to define-
>> configuration, because a lot of this code would go away. But I need
>> to explore how define-configuration works.
>> That would be quite a major change. :)
> 
> Manchmal erspart einem monatelange Implementier-Arbeit einen Nachmittag
> in der Bücherei.
> 
> Cheers
> 
> [1] https://issues.guix.gnu.org/issue/56046#4-lineno323

Previous Next

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