4

Sometimes it is useful to cast function callbacks without.

For example, we may have a function to duplicate some data:
struct MyStruct *my_dupe_fn(const struct MyStruct *s)

But pass it as a generic callback:
typedef void *(*MyGenericCopyCallback)(void *key);

Eg:
ensure_key_in_set(my_set, my_key, (MyGenericCopyCallback)my_dupe_fn);

Since the difference between const struct MyStruct * and void * is not going to cause problems in this case, it won't cause any bugs (at least in the function call its self).

However, if later on an arguments added to my_dupe_fn, this could cause a bug which wouldn't give a compiler warning.


Is there a way to cast a function, but still show warnings if the arguments or return values are different sizes?



Obligatory disclaimer: of course C isn't *safe*, but ways to prevent potential bugs in a widely used language are still useful.

ideasman42
  • 42,413
  • 44
  • 197
  • 320
  • you could write ``my_dupe_fn`` with the signature of the typedef'ed "contract" and cast your args inside the function to your required type. Then you would not need the cast and you would be "robust" regarding those potential future changes. A good example for this is the ``DWORD ThreadProc( void * parameter)``. I never saw anyone trying to cast a differently typed function to the generic version. Usually people cast parameter to their expected type inside ``ThreadProc``. – BitTickler May 10 '15 at 23:17
  • 1
    There are few ways to prevent someone from doing something stupid in C, and this isn't one of those few. That is somewhat the point of a *cast*, to forcibly tell the compiler "I'm on this, no worries on your side". Any code pointer can be forced to wear that disguise. Ultimately it is the *caller's* responsibility to provide the correct interface. If they force a cast, *they* dropped the ball. They should be writing a proper matching function and *not* casting their fptr *precisely* to allow catching this situation when the interface changes. – WhozCraig May 10 '15 at 23:30
  • Not sure, but: is the cast of `my_dupe_fn` really required? void * can be assigned to any other pointer type without cast, but I'm not sure if this works with this indirection. – too honest for this site May 11 '15 at 00:22
  • @Matt McNabb, `typedef void *(*MyGenericCopyCallback)(void *key);` is just a typedef for a function signature. – ideasman42 May 11 '15 at 02:03
  • @Matt McNabb, it doesnt have to be defined anywhere, its common enough C code. See `compfn` - https://support.microsoft.com/en-us/kb/73853 – ideasman42 May 11 '15 at 02:09
  • argh indeed, my bad, was meant to be `MyGenericCopyCallback` – ideasman42 May 11 '15 at 05:04

4 Answers4

3

You say "won't cause any bugs", however it causes undefined behaviour to call a function through a function pointer with incompatible return types or parameter types, even in your example code.

If you want to rely on undefined behaviour then that's your risk to take. Relying on UB has a tendency to cause bugs sooner or later. A better idea would be to re-design the callback interface to not rely on undefined behaviour. For example, only use functions of the correct type as the callback function.

In your example this might be:

typedef void *MyCallback(void *key);    // style: avoid pointer typedefs

struct MyStruct *my_dupe_fn(const struct MyStruct *s)
{ ... }

void *my_dupe_fn_callback(void *s)
{
     return my_dupe_fn(s);
}

void generic_algorithm(MyCallback *callback)
{
    // ....
    ensure_key_in_set(my_set, my_key, callback); 
    // ....
}

// elsewhere
generic_algorithm(my_dupe_fn_callback);  

Note the lack of casts. Managing a style policy of not using any function casts is simpler than a policy of allowing certain types.

M.M
  • 138,810
  • 21
  • 208
  • 365
  • Well, that is, however pretty hard to read/undestand. Expecially in non-trivial code. – too honest for this site May 11 '15 at 00:24
  • @Olaf what's hard to read or understand? (I had to guess a lot of details about the example since it was pretty threadbare) – M.M May 11 '15 at 01:30
  • The point in the original question is that the function call its self wouldn't cause any bugs. Of course internal structure of the args or use of the return value - still has the possibility of an error. – ideasman42 May 11 '15 at 01:59
  • 1
    @ideasman42 It would cause [undefined behaviour](http://stackoverflow.com/a/4105123/1505939). If you claim that some UB is "not a bug" then you are on very thin ice – M.M May 11 '15 at 02:04
  • Just think big and about quite some of these trampoline functions. "Hard to read" might have been a bit too harsh, but it certainly does not contribute to readability. But, granted, I hav not better standard-conformant solution out at hand right right now. – too honest for this site May 11 '15 at 02:19
2

If you are using gcc and are not afraid of using helpful extensions, you might have a look at plan9-extensions. In combination with anonymous struct fields (standard since C99) as the first field, they allow to build a type-hierarchy with static functions, etc. Avoids tons of casts in my code and makes it much more readable.

Not sure, but according to the gcc documentation, the MS-compiler supports some (all?) these features, too. No warranty for that, however.

too honest for this site
  • 12,050
  • 4
  • 30
  • 52
1

That later error is coming from two pieces of code that say the same thing getting out of sync -- the first where you define the type of my_dupe_fn, and the second where you cast the generic callback pointer back to its original type.

This is where DRY (do not repeat yourself) comes in. The whole point is to only say something once, so that you can't later come back and change only one instance.

In this case, you'd want to typedef the type of a pointer to my_dupe_fn, preferably very close to where you declare the function itself, to help ensure that the typedef always changes along with the function signiture itself.

The compiler is never going to catch this for you as long as it thinks that it is just dealing with a generic void pointer.

Barry Gackle
  • 829
  • 4
  • 17
  • 1
    While this is a valid answer, its not quite right to suggest that the compiler will *never* going to catch this. Theres all sorts of valid C code which compilers can (optionally) warn about, because they're often the cause of errors. – ideasman42 May 10 '15 at 23:30
  • 1
    Very true. There are definately ways to automate checking for something like this in practice. What I meant to convey, though, is that C lacks language guarantees for figuring something like this out -- ie, I'm not aware of a way to write this so that the mistake would in fact be invalid C code (I would love to be proven wrong on that point, though, since this would be super useful). – Barry Gackle May 11 '15 at 00:09
  • C has no strong typing. Most non-trivial program has to fiddle around somehow with typecasts and rely on the correct type. Standard example is malloc: the programmer has to ensure the result-type matches the receiving pointer's type (i.e. the size fo malloc and the pointer type match). – too honest for this site May 11 '15 at 00:15
0

Unfortunately you typically have to forgo some of this compile-time safety if you're using C. You might get a warning at best, but if you have a design that is uniformly casting function pointers this way, you're likely to ignore or outright disable them. Instead you want to place your emphasis on achieving safe coding standards. What you can't guarantee by force, you can encourage strongly by policy.

I would suggest, if you can afford it, to start by casting arguments and return values rather than whole function pointers. A flexible representation is like so:

typedef void* GenericFunction(int argc, void** args);

This emulates the ability to have variadic callbacks, and you can uniformly do runtime safety checks in debug builds, e.g., to make sure that the number of arguments matches the assumptions:

void* MyCallback(int argc, void** args)
{
    assert(argc == 2);
    ...
    return 0;
}

If you need more safety than this for the individual arguments being passed and can afford a typically-small cost of an extra pointer per argument with a slightly bulky structured solution, you can do something like this:

struct Variant
{
    void* ptr;
    const char* type_name;
};

struct Variant to_variant(void* ptr, const char* type_name)
{
    struct Variant new_var;
    new_var.ptr = ptr;
    new_var.type_name = type_name;
    return new_var;
}

void* from_variant(struct Variant* var, const char* type_name)
{
     assert(strcmp(var->type_name, type_name) == 0 && "Type mismatch!");
     return var->ptr;
}

void* pop_variant(struct Variant** args, const char* type_name)
{
     struct Variant* var = *args;
     assert(var->ptr && "Trying to pop off the end of the argument stack!");
     assert(strcmp(var->type_name, type_name) == 0 && "Type mismatch!");
     ++*args;
     return var->ptr;
}

With macros like so:

#define TO_VARIANT(val, type) to_variant(&val, #type);
#define FROM_VARIANT(var, type) *(type*)from_variant(&var, #type);
#define POP_VARIANT(args, type) *(type*)pop_variant(&args, #type);

typedef struct Variant* GenericFunction(struct Variant* args);

Example callback:

struct Variant* MyCallback(struct Variant* args)
{
    // `args` is null-terminated.
    int arg1 = POP_VARIANT(args, int);
    float arg2 = POP_VARIANT(args, float);
    ...
    return 0;
}

A side benefit is what you can see in your debugger when you trace into MyCallback through those type_name fields.

This kind of thing can be useful if your codebase supports callbacks into dynamically-typed scripting languages, since scripting languages should not be doing type casts in their code (typically they're meant to be a bit on the safer side). The type names can then be used to automatically convert the arguments into the scripting language's native types dynamically using those type_name fields.