`fctprintf` needs void pointer argument

[sgoll]

For fctprintf to be more useful, it would be great if it had a void pointer argument to customize what the given callback function actually relates to, e.g. with regard to a specific stream instance.

Right now, only a statically defined function can be passed to fctprintf which must behave identical (bar global variables which would defeat the whole purpose of having the callback) each call.

Think about the following signature and call examples:

int fctprintf(void (*out)(char character, void *user), void *user, const char *format, ...);

struct Stream
{
  // ...
};

void do_out(char character, void *user)
{
  Stream *stream = (Stream *)user;
  // Output character on `stream`.
}

Stream stream_1;
Stream stream_2;

fctprintf(do_out, (void *)&stream_1, "%s", "Hello 1");
fctprintf(do_out, (void *)&stream_2, "%s", "Hello 2");

[mpaland]

You are absolutely right! I'm a big fan of void* arguments passed to callback functions.
In the case above, I just thought it won't be necessary, but your example is a use case.
So, I merge your PR shortly. THANKS!

[mpaland]

Merged!
I renamed param 'user' to 'arg' for more common nomenclature.

[vgrudenic]

Hi guys! I have an additional question (suggestion) regarding @sgoll's merge.

The starting reason why I came to this is that on ARM GCC with -Wcast-align (which is one of the additional warnings parameters we use with ARM to avoid alignment issues) we get an alignment warning when casting the char* to a struct pointer in this function:

static inline void _out_fct(char character, char* buffer, size_t idx, size_t maxlen)
{
  // warning: cast from char* to out_fct_wrap_type* increases
  //          required alignment of target type
  
  ((out_fct_wrap_type*)buffer)->fct(character, ((out_fct_wrap_type*)buffer)->arg);
}

Although it isn't the case here (buffer always points to an actual struct instance when this function is called), the warning would generally make sense according to the C standard (6.3.2.3 §7):

If the resulting pointer is not correctly aligned for the referenced type, the behavior is undefined.

So the cast itself is not undefined behavior, unless the pointer is misaligned at runtime, which it isn't.

But anyway, at first I thought of using a #pragma to disable the warning, but then I also noticed that (as of this revision) the buffer parameter is actually a "special case", used only inside _out_buffer and it seems like there is no need to use it everywhere.

That is, writing to char* buffer is actually just a special case of the function call where the newly introduced void* arg would point to char* buffer, and the _out_buffer function would just cast it to char*, while _out_char would again simply ignore the arg part completely.

To clarify, this would require replacing the char* pointer with void*:

// char* buffer becomes void* arg, because this will work the same way for fctprintf    
typedef void (*out_fct_type)(char character, void* arg, size_t idx, size_t maxlen);

// out_fct_wrap_type takes this same signature
typedef struct {
  out_fct_type fct;
  void* arg;
} out_fct_wrap_type;

And then all calls to out(...) would be changed from

static int _vsnprintf(out_fct_type out, char* buffer, ...
{
  ...
  out(character, buffer, idx++, maxlen);
  ...
}

to

// no need to pass `out_fct_type` and `char*` separately to this function,
// because `out_fct_wrap_type` wraps them both
static int _vsnprintf(out_fct_wrap_type out, ...
{
  ...
  out.fct(character, out.arg, idx++, maxlen);
  ...
}

And then individual "overloads" of printf would change from:

int printf(const char* format, ...)
{
  ...      
  char buffer[1];
  const int ret = _vsnprintf(_out_char, buffer, (size_t)-1, format, va);
  ...
}

to something like this:

int printf(const char* format, ...)
{
  ...
  const out_fct_wrap_type out_fct_wrap = { _out_char, NULL };
  const int ret = _vsnprintf(out_fct_wrap, (size_t)-1, format, va);
  ...
}

Does this make sense? There would be no changes to the public interface, except the func signature in fctprintf (which perhaps isn't a bad thing because it lets the caller get the idx too).

Sorry for potential typos. If you are interested and don't think that this will change the code too much (shouldn't change the public interface though), I can make a PR.

Thanks!

[mpaland]

Hello Vedran, thanks A LOT for your detailed description and bringing -Wcast-align to attention.
You got a point here, obviously the char* pointer is always correct aligned when pointing to the wrapper struct, but I agree, it's not 100% clean regarding the standard.

Suppressing warnings may help in certain situations but is not the intention here, because this module should work rocksolid in any environment.

I think, you are right, replacing the char* pointer by a void* might solve the problem. Going to investigate this...