Solar Designer wrote:
[]
> > char defbuf[256]; <== some not very large but still enouth
> > for most cases size
> > char *buf = defbuf;
> > int size = sizeof(defbuf);
> > while (snprintf(buf, size, ...) cant fit in size) {
>
> The "can't fit" is ">= size" with non-broken snprintf()'s. I don't
> think we should support broken ones at all, simply use a reasonable
> size by default and, if that isn't enough, a truncated log entry is
> the price paid for using a broken libc. ;-)
I didn't mean broken snprintf/etc, maybe I just improperly
written that condition. I mean "while(size isn't enouth for
formatted string of full length)".
> > size *= 2;
> > buf = (buf == defbuf) ? malloc(size) : realloc(buf, size);
> > }
>
> Yes, this is what I meant. I don't think you need that one buffer on
> the stack, -- it only adds complexity. You also need a free().
it adds minimal complexity and removes unnecessary for 99.9% cases
malloc/free pair.
if (buf != defbuf) free(buf);
But that's at least "non-essential" thing. ;)
[pam_log vs pam_log & pam_vlog]
>
> What does this buy you? I think pam_log() can be simple enough.
Sometimes it is very nice to have two-args log(format, va_list) --
this can simplify code in (some) places gracefully.
One example if some sort of "macro" like this (in module):
static inline void _pam_debug(int flags, pamh, char *fmt, ...) {
if (flags & F_DEBUG) {
va_list ap; va_start(ap, fmt);
pam_log(pamh, LOG_DEBUG, fmt, ap);
va_end(ap);
}
}
This hides tons of really unnecessary code like the test (flags & F_DEBUG)
that looks just ugly in places where you do real work.
To implement that _pam_debug() as a macro, you need gcc-style
"macro varargs", and no other way exists for that trivial task.
(#define _pam_debug(flags,pamh,arg...) \
((void)((flags)&F_DEBUG?pam_log(pamh,LOG_DEBUG,arg):0))
)
pam_log+pam_vlog _is_ as simple as single pam_log, it just split
onto two parts, that's all.
[]
