[syslog-ng] [PATCH] tfhash: New template function to calculate the SHA1 of its input

[Gergely Nagy]

Balazs Scheidler <bazsi77 at gmail.com> writes:

> Having now actually reviewed the code in question, I'd have some remarks:
>
> 1) template functions should rather use command-line parsing than
> positional arguments. (e.g. I'd prefer --method, --length, etc as
> arguments instead of hardcoding the order)

Well, in general, I would agree, but not in this case: for $(hash), the
method is mandatory - at least in the version I made from Peter's
original -, so is the string, length is the only optional argument, and
$(hash --method SHA1 --length 8 $BLAH) is longer than $(hash SHA1 $BLAH
8) and is not easier to understand, either, as far as I'm concerned.

When there are many more optional arguments, then --foo is superior
indeed. But when there's only one... not so much. (substr uses
positional arguments too, so do many other template functions,
format-json being the most visible exception as far as I see, and due to
value-pairs, that has a lot of optional arguments)

I'll see if I can prepare a patch that adds command-line parsing,
though. I won't collapse that into a single commit, however, in hopes I
can persuade you to keep the current syntax :)

> 2) also, it should support a series of positional arguments, which in
> turn should be concatenated when producing the hash

$(hash SHA1 "$HOST $PROGRAM") would already do the right thing, no?

But agreed, it's less suprising if it works like you described. I
already pushed out a feature/3.4/cryptofuncs branch, but I'll push a
patch on top Soon(tm) that implements the automatic concatenation too. I
might end up rebasing that branch.

> 3) basicfuncs shouldn't link to the crypto library, rather a new
> cryptofuncs module should be introduced, that links against
> libsyslog-ng-crypto.so, that module can then include the $(uuid)
> function too.

Done, patch on my feature/3.4/cryptofuncs branch. I had to move
tf_parse_int() out of basicfuncs, into lib/misc.c, so that both
basicfuncs and cryptofuncs can make use of it. And tfuuid was merged
into this new module, of course.

-- 
|8]