On 07.01.2014 19:55, Simo Sorce wrote:
A few notes:
1. style
although we allow things like
if (condition)
statement;
I do not think we accpet for or while loops w/o braces
Nod.
2. the functions are incomplete and lack documentation that states
so
in particular the only accepted format type is 's' (string).
I think we want a complete replacement for [a|sn]printf if you want to
put it in ding-libs.
If you do not want to make the general puprpose version then I propose
you contribute this only as a private function for sssd, but please add
documentation in the header file about what formatting is supported.
The reason that the printf positional arguments are so brittle (one must
consume them all) is exactly because arguments of different sizes are
accepted in the varargs. So obviously the positional code cannot know
the relevant vararg if some are not consumed.
Having the only accepted format type be 's' follows from that. Otherwise
it would be difficult to have a user-provided format string.
But yes, general purpose is not the goal. Fixing sssd to be robust and
correct is the goal. I'd much rather not put this sorta thing in general
purpose library like ding-libs.
3. can you change from 'callback' to 'copy_fn' ?
Sure.
On the code, looks mostly ok except the incompleteness of the printf
function, however:
On Tue, 2014-01-07 at 16:07 +0100, Stef Walter wrote:
> +static void
> +snprintf_callback (void *data,
> + const char *piece,
> + size_t length)
> +{
> + struct sprintf_ctx *cx = data;
> +
> + /* Don't copy if too much data */
> + if (cx->length > cx->alloc)
> + length = 0;
> + else if (cx->length + length > cx->alloc)
> + length = cx->alloc - cx->length;
> + else
> + length = length;
The last one branch is unnecessary and a bit useless :-)
Oops. Refactoring strikes again...
Stef