Hi Max,
On Wed, Sep 03, 2025 at 02:09:44PM +0200, Maximilian Moehl wrote:
> This commit adds the base2 converter to turn binary input into it's
> string representation. Each input byte is converted into a series of
> eight characters which are either 0s and 1s using the new base2tab array
> which maps every possible value of a byte to its string representation.
Cool, thanks for doing this!
I do have an objection however on a part:
> +static const char base2tab[256][8] = {
> + "00000000", "00000001", "00000010", "00000011", "00000100", "00000101",
(...)
This will inflate the binary by 2kB and will cause L1 cache thrashing
for something that we can easily compute on the fly. Also one minor nit
is that some modern compilers will complain that the strings are not
zero-terminated unless you use [9]. A trick could also be to concatenate
all of them into an array of 2049 chars read by packs of 8 but still...
> +static int sample_conv_bin2base2(const struct arg *arg_p, struct sample
> *smp, void *private)
> +{
> + struct buffer *trash = get_trash_chunk();
> + unsigned char c;
> + int ptr = 0;
> +
> + trash->data = 0;
> + while (ptr < smp->data.u.str.data && trash->data <= trash->size - 8) {
> + c = smp->data.u.str.area[ptr++];
> + memcpy(trash->area + trash->data, base2tab[c], 8);
> + trash->data += 8;
> + }
So the approach here is to encode 'c' to binary. For this you just
need to visit bit positions from 7 to 0 and emit 0 or 1. For example
like this (not tested, but just to give you an idea):
int bit;
while (ptr < smp->data.u.str.data && trash->data <= trash->size - 8) {
c = smp->data.u.str.area[ptr++];
for (bit = 7; bit >= 0; bit--)
trash->area[trash->data++] = c & (1 << bit) ? '1' : '0';
}
See the principle ? It can be done in a multitude of other variants
that are all equally acceptable (and just a matter of taste). But
this way we don't need to store pre-computed tables of numbers that
the computer can calculate extremely cheaply.
Otherwise your patch looks pretty good to me in terms of quality, style,
documentation and commit message, that's excellent for a first submission!
Please give the change above a try (and/or fix it if I messed up with
anything), and I'm definitely willing to take this patch!
Thank you!
Willy
