[systemd-devel] [PATCH 3/3 (rebased)] cryptsetup: Add tcrypt support

Lennart Poettering lennart at poettering.net
Fri Jul 12 12:14:46 PDT 2013


On Fri, 12.07.13 20:48, Jan Janssen (medhefgo at web.de) wrote:

> 
> On 07/12/2013 08:36 PM, Lennart Poettering wrote:
> >On Tue, 09.07.13 21:15, Jan Janssen (medhefgo at web.de) wrote:
> >
> >>+        if (*key_file) {
> >>+                r = read_one_line_file(*key_file, &passphrase);
> >>+                if (r < 0) {
> >>+                        log_error("Failed to read key file: %s", strerror(-r));
> >>+                        *key_file = NULL;
> >>+                        return -EAGAIN;
> >
> >I can't say I like functions that change the parameters when they fail,
> >any chance we can fix that?
> >
> >otherwise looks good.
> >
> >Lennart
> >
> 
> When I read the (old) luks code correctly, it does the same: falling
> back to normal password query if the key file does not work. I just
> thought it would be best to do the same here.

I am all for falling back to normal password query if the file does not
exist, however, I don't like the programming style where you alter
pass-by-reference parameters of function when failing.

Changing pass-by-ref parameters on success is a good way to return data,
but I think it is generally a bad idea to alter pass-by-ref params if
you fail. 

So, this is not a question of general behaviour of the tool, but only of
the coding style. 

Lennart

-- 
Lennart Poettering - Red Hat, Inc.


More information about the systemd-devel mailing list