Refactor print_sep()

[gperciva]

No description provided.

[gperciva]

We previously had 2 different versions of print_sep(), and I had to write another one to implement --print-stats --null-output.

That stuck me as sub-optimal, so I've merged the functionality into a print_separator() function. To simply its usage, I defined print_sep() macros in each .c file which used it.

[cperciva]

I like the idea of print_separator.c but I'm not convinced about having all of the print_sep macros with various parameters hard-coded. Is this horribly ugly if we drop those macros and have calls to print_separator everywhere?

[gperciva]

Depends on your definition of "horribly ugly".

/* Print arg separator. */
if ((arg > 0) && print_sep(" ", 3))
        goto err1;

vs.

/* Print arg separator. */
if ((arg > 0) && print_separator(stdout, " ",
    print_nulls, 3))
        goto err1;

Also,

print_field("%15s", "Total size", err0);
print_sep("  ", 2, err0);
print_field("%15s", "Compressed size", err0);
print_sep("\n", 1, err0);

vs.

print_field("%15s", "Total size", err0);
if (print_separator(stream, "  ", print_nulls, 2))
        goto err0;
print_field("%15s", "Compressed size", err0);
if (print_separator(stream, "\n", print_nulls, 1))
        goto err0;

To my eye, using print_sep() makes it much easier to see details like " ", 2 vs. "\n", 1.

That said, we're not going to be editing chunks_stats_internal.c very often, and as long as we're comparing print-stats output byte-for-byte (which I'm now doing), any errors will be found immediately. So if you prefer print_separator() directly, then I have no further objection.

[gperciva]

... oh wait, in that last example, if you don't like the print_sep macro, then you probably don't like print_field either. So the "horribly ugly" version would be:

if (fprintf(stream, "%15s", "Total size"))
        goto err0;
if (print_separator(stream, "  ", print_nulls, 2))
        goto err0;
if (fprintf(stream, "%15s", "Compressed size"))
        goto err0;
if (print_separator(stream, "\n", print_nulls, 1))
        goto err0;

That seems to be "unnecessarily busy" to me... but again, we're not going to be changing this function a lot, so I have no further objection if you prefer it this way.

[gperciva]

Added a REBASE commit to show it without the print_sep() macros.

[cperciva]

Yeah I prefer it without print_sep macros. Easier to understand, and as you say we probably won't be touching this code again any time soon.

[gperciva]

Ok, revised accordingly.