Nikola,
thanks for your contribution. I agree with Tim's points, and that's a
good indicator of the quality of your patch, since all of this is purely
cosmetic :-)
I do have one minor cosmetic comment to add here:
+static int sample_conv_add_item(const struct arg *arg_p, struct sample *smp,
void *private)
+{
+ struct buffer *trash;
+ struct sample tmp;
+ int max, v_flag;
+
+ trash = alloc_trash_chunk();
+ if (!trash)
+ return 0;
Please use a different name from "trash" for this dynamic trash. Call
it "tmpbuf", "trash2" or whatever you see fit. We've done this mistake
a few times in the past to call such dynamic buffers "trash" but the
problem is that it causes confusion when reading the code because
"trash" is a well-known thread-local global variable and it's easy to
confuse one with the other. Actually the other places where the "trash"
name is used like this happened because a conflict forced us to dyanmically
allocate a trash to replace the current one.
{ "regsub", sample_conv_regsub, ARG3(2,REG,STR,STR),
sample_conv_regsub_check, SMP_T_STR, SMP_T_STR },
{ "sha1", sample_conv_sha1, 0, NULL,
SMP_T_BIN, SMP_T_BIN },
{ "concat", sample_conv_concat, ARG3(1,STR,STR,STR),
smp_check_concat, SMP_T_STR, SMP_T_STR },
+ { "add_item",sample_conv_add_item, ARG3(2,STR,STR,STR),
smp_check_concat, SMP_T_STR, SMP_T_STR },
{ "strcmp", sample_conv_strcmp, ARG1(1,STR),
smp_check_strcmp, SMP_T_STR, SMP_T_SINT },
Please place it at the top to fix the alphabetical order, and feel free
to move concat with it as it was obviously misplaced.
Other than that, it looks pretty good, thank you very much for taking
care of this useful feature!
Willy
