On Wed, Sep 03, 2014 at 04:41:40PM +0200, Tomas Mraz wrote:
On St, 2014-09-03 at 18:07 +0400, Dmitry V. Levin wrote:
> On Wed, Sep 03, 2014 at 03:33:57PM +0200, Tomas Mraz wrote:
> [...]
> > Well at least the pam module name cannot be /.so but grantor=/ could be
> > confusing as well. I'll stick with the original grantor=? as supporting
> > module named ?.so is something we do not really have to :).
>
> Well, I've never had a module named ?.so before, but I have it now,
> and it works perfectly well. :)
>
> > So, I'll rather change the patch so there is no possibility of the
> > message not having the grantor field than remove the grantor=? case
> > completely.
>
> OK, maybe call it "/?", so that the name will be distinguishable from
> supported module names?
>
> > The attached patch should contain this change and all the other minor
> > corrections.
> [...]
> > + if (asprintf(&buf, "PAM:%s%s%s", message, grantors?"
grantors=":"", grantors?grantors:"") >= 0) {
>
> Could you kindly make this expression more readable, please?
Do you accept just whitespace changes or do you want me also to replace
the ? expressions with ifs and variable assignments?
Whatever you like, just don't make new lines too long or too dense to read.
--
ldv