Macro functions in headers

[PlainName]

Title doesn't really cover it properly, so here is what it is:

I have a C module which has a lot of functions which all basically do the same thing apart from using different variables. They are get/set functions for the config, reading and writing NVS. So the string functions might look like this (it is ESP-IDF):

Code: [Select]

void confSetURL(char *url)
{
    nvs_set_string(hconfig, NVS_KEY, url);
    nvs_commit(hconfig);
}
OK, so I am writing a macro that saves me typing that all out:

Code: [Select]

#define confSetStr(NAME, KEY) \
void confSet##NAME(char *str) \
{ \
nvs_set_str(hconfig, KEY, str); \
nvs_commit(hconfig); \
}
and I can then do lots of line like:

Code: [Select]

confSetStr(URL, CFG_KEY_URL);
and call it:

confSetURL("www.google.com");

Except, I have to manually add the real function prototype to the .h in order to achieve that, and I am now looking at not just typos but a non-correlation between header prototype and macro instantiation.

Any way around this?

(I have no doubt someone will say that this - the many nearly identical functions - is not the way to do this, but it's the getting callable prototypes into the header that I'm really interested in here.)

[langwadt]

maybe something like the last answer here?

https://stackoverflow.com/questions/49099976/how-do-you-define-functions-in-header-files

[SiliconWizard]

Several ways to deal with it. Two of them:

- Either define an additional macro to generate the prototypes. Would just be:

Code: [Select]

#define confSetStr_Proto(NAME)    void confSet##NAME(char *str)And then in the header file, you'd have to declare the prototypes like so:

Code: [Select]

confSetStr_Proto(URL);
- Or you could define the functions themselves inside the header file. You just need to qualify them static. The added benefit is that they'll get inlined everywhere they'll be called when optimizations are enabled. The drawback is that: possibly excess code size if the functions in question are large, but in the case of very small functions like getters and setters, that's IMHO a good approach:

Code: [Select]

#define confSetStr(NAME, KEY) \
static void confSet##NAME(char *str) \
{ \
nvs_set_str(hconfig, KEY, str); \
nvs_commit(hconfig); \
}And then in the header file, you'd have to define the functions like so:

Code: [Select]

confSetStr(URL, CFG_KEY_URL)(Note that you can't add a final semicolon as you did in your example for defining functions with a body. The semicolon right after the final brace of the function's body will normally give a syntax error.)

[brucehoult]

Put all the calls to confSetStr() in a .inc file (name doesn't matter, it's just a convention) and then #include it into both the header and the C file, with different definitions of the macro in each (and #undef it after)

As I do here:

https://github.com/brucehoult/trv

[PlainName]

maybe something like the last answer here?

https://stackoverflow.com/questions/49099976/how-do-you-define-functions-in-header-files

I couldn't see how that was relevant, sorry.

[PlainName]

- Either define an additional macro to generate the prototypes. Would just be:

Code: [Select]

#define confSetStr_Proto(NAME)    void confSet##NAME(char *str)And then in the header file, you'd have to declare the prototypes like so:

Code: [Select]

confSetStr_Proto(URL);

That has possibilities, yes. Hmmm.

- Or you could define the functions themselves inside the header file.

I am reluctant to do that because of potential coupling. The calling code would need to know of, and be calling, nvs_set_str(), and know of hconfig, etc.

The drawback is that: possibly excess code size if the functions in question are large

The getter is bigger, albeit not by a lot. But I don't think doing this (putting it in the header) would change things since this is all really just saving typing and source size - there would still be the duplicated functions with just differerent names and key variables.

[Nominal Animal]

Do you really need to define a set of functions to perform the functionality?  Why not use

Code: [Select]

#define  confSet(key, value)  do { nvs_set_str(hconfig, NVS_##key, value); nvs_commit(hconfig); } while (0)
instead, in only the header file?  This way, there is only one "function" to call.  Each confSet() is preprocessed into a pair of calls: one to nvs_set_str(), and the other to nvs_commit().  See how the NVS_ prefix is automagically added to the key?  This limits the key namespace to constants and macros that begin with NVS_.

I'm pretty sure confSet(URL, value); is just as easy to use as confSetURL(value); is.

Personally, I'd do the following instead:

Code: [Select]

#define  confAdd(key, value)  nvs_set_str(hconfig, NVS_##key, value)
#define  confCommit()  nvs_commit(hconfig)
so that in your code, you have one or more confAdd() lines, followed by a confCommit().  For example,

Code: [Select]

    confAdd(URL, CFG_KEY_URL);
    confAdd(FOO, CFG_KEY_FOO);
    confAdd(BAR, CFG_KEY_BAR);
    confCommit();
instead.  You then put say confAdd(URL, whatever); confAdd(FOO, foovalue); confAdd(BAR, barvalue); confCommit(); in the code.

[PlainName]

Put all the calls to confSetStr() in a .inc file (name doesn't matter, it's just a convention) and then #include it into both the header and the C file, with different definitions of the macro in each (and #undef it after)

As I do here:

https://github.com/brucehoult/trv

That certainly has potential, thanks 

[Nominal Animal]

The calling code would need to know of, and be calling, nvs_set_str(), and know of hconfig, etc.

To hide those, expose the common call in the header file,

Code: [Select]

void confSet(int key, const char *val);
You do need to expose the NVS_ constants somehow – but they don't need to have the same names, just the corresponding integer values.  So, in addition to the above, the header file does need

Code: [Select]

#define  CONFSET_URL  the_same_value_as_NVS_URL_has
and so on.

This way the "user" code can do confSet(CONFSET_URL, the_url); .

Then, in the internal implementation that does have access to hconfig etc, define

Code: [Select]

void confSet(int key, const char *val)
{
    nvs_set_str(hconfig, key, str);
    nvs_commit(hconfig);
}
This one must have global linkage (not be static), so that it can be called from the "user" code.

[PlainName]

Do you really need to define a set of functions to perform the functionality?

No, however it can be done opaquely is OK. But there are things internal to the config module that shouldn't be visible outside (hconfig, for instance, and the commit).

Edit: sorry, you overtook me as I was typing

You do need to expose the NVS_ constants somehow – but they don't need to have the same names, just the corresponding integer values.

Yes. Hmmm. Well this is another aspect which I haven't actually decided on yet, whether discrete functions for the data is sensible or should there be a single function and an ID key. On a small scale the discrete functions are nice because the prototype can be commented, whereas using an ID it is a bit trickier because it covers many different issues. However, if there were, say, 50 of the things it's wasting code space.

[langwadt]

maybe something like the last answer here?

https://stackoverflow.com/questions/49099976/how-do-you-define-functions-in-header-files

I couldn't see how that was relevant, sorry.

put the functions in the header file so you don't need the prototypes and use __attribute__((weak)) so you only get one copy of each function

[PlainName]

Ah, OK. As you've no doubt seen since, I was reluctant to put the function body in the header. Bu thanks for the clarification

[SiliconWizard]

- Or you could define the functions themselves inside the header file.

I am reluctant to do that because of potential coupling. The calling code would need to know of, and be calling, nvs_set_str(), and know of hconfig, etc.

Unless your concern here is to "hide" this particular information from the caller, it's just a matter of including the required corresponding prototypes and external definitions, in another header file, inside this one header file.

[PlainName]

Sure, but it's like leaving live wires hanging from the roof - of course you're not going to grab them so they're not a problem. But we wouldn't be seen dead (sorry!) doing it.

[Nominal Animal]

If you go by the numeric key route, here is one "trick" that might be useful.

Instead of declaring the keys only as preprocessor macros, you declare them as enums instead:

Code: [Select]

enum {

         CONF_URL = 1, /*!< Main configuration URL string */
#define  CONF_URL  CONF_URL

         CONF_FOO, /* !< Some feature named FOO */
#define  CONF_FOO  CONF_FOO

};
In C, these (CONF_URL and CONF_FOO) are ints.  In C++, you can use enum typename: integer-type { ... }; instead, so that you get a new type named typename that is of integer-type, say uint8_t for example.

Instead of maintaining these in that form, you can make a file in your preferred format with three fields per record: name-suffix, integer-value (or a marker for auto-numbering, like CONF_FOO is above), and comment-description, and then use a script to generate a pair of lines for each.  If the header file then contains

Code: [Select]

enum {
#include "generated-enums.inc"
};
with the script output saved to generated-enums.inc, you can include the generation in your Makefile-based build facilities very easily.  (Just make generated-enums.inc a prerequisite of the target header file, with the rule running the generator script when necessary.  If you ever add proper dependency tracking (make dep) so that all affected files get recompiled when any source file gets modified, Everything Just Works, this way.)

If you want, I can easily construct a small example, but do remember I use Linux.  (So state your preferred language for the generator: bash, sh, awk, sed, perl, python.  Since the generator is run on the host, it makes sense to use a scripting language instead of C, because when cross-compiling to a different architecture, you may not even have a C toolchain that can generate code to run on the host itself.)

[Ian.M]

It should be possible to avoid the need for OS specific generator scripts by using X-macros: https://en.wikibooks.org/wiki/C_Programming/Preprocessor_directives_and_macros#X-Macros

[PlainName]

Instead of declaring the keys only as preprocessor macros, you declare them as enums instead:

I always do key-type stuff as typedef enums.

Er, except for these because they're actually strings

[PlainName]

It should be possible to avoid the need for OS specific generator scripts by using X-macros

Thanks. Looks like the thing that Bruce described - not heard the term xmacro before, but it's descriptive.

[Nominal Animal]

Instead of declaring the keys only as preprocessor macros, you declare them as enums instead:

I always do key-type stuff as typedef enums.

Er, except for these because they're actually strings

That's one use where I do use the above-kind of header-file generation: collecting a set of strings into a hash table (usually using DJB2-xor variant, so very, very fast) of minimal size; one that is mapped and verified at build time.

That way you could only expose int confSet(const char *key, const char *val); but still very efficiently parse the keys.  On an ESP32, the run time difference between an integer key and a string key would then be absolutely neglible, and the only downside be the additional ROM/Flash use (for the key strings and associated little bit of data).

I like to do this for (embedded) configuration parsing.  Then, the hash table strings are configuration keys, and the values are pointers to the data, and the value-parsing function and its parameters in an union, so that although each configuration value is parsed using a single function, there only needs to be one per parameter type, instead of one per parameter.

The nice thing is that everything is trivially verified to be okay at build time, and the interface is always easy to extend: there is no risk of accidentally changing the keys (as there is when enumerating them), because the old keys always stay as they are.  Only the firmware hash table varies, but since it is only needed to accelerate the finding of the matching key, it's perfectly okay.

[SiliconWizard]

I would also do this with a single function. And yes the difference would be absolutely negligible. Unless you were going to call those functions extensively in time-constrained parts of the code, which I doubt here.

[SiliconWizard]

Sure, but it's like leaving live wires hanging from the roof - of course you're not going to grab them so they're not a problem. But we wouldn't be seen dead (sorry!) doing it.

Frankly, unless you're writing an API for others to use, or are expecting this piece of code to be maintained by many developers over the next decades, it won't make a difference.
Just because it would be exposing those helper functions to the caller doesn't mean you have to use them. Nothing is hanging. As much as I like hiding and encapsulating, you're probably overthinking it here.

That said, all in all I prefer Nominal Animal's approach for your need here. My proposal is more adapted to functions that do not have external dependencies, and that are possibly called very frequently - and most of all, that potentially all have a different argument list. In your case, it's always the same and only the name of the function differs, which makes Nominal's approach easier and a lot more consistent.

[brucehoult]

It should be possible to avoid the need for OS specific generator scripts by using X-macros

Thanks. Looks like the thing that Bruce described - not heard the term xmacro before, but it's descriptive.

Ha! Exactly the same, yes.

I didn't know it had a name. Or that someone had done it before (but it's unsurprising).

Preprocessor ab-uuuuuuse

I'm pretty certain the ugly hacky preprocessor is 90% of the reason C beat the Wirthian languages outside of the Unix environment.

80% of early C++ was getting rid of preprocessor hacks, one by one.

[SiliconWizard]

80% of early C++ was getting rid of preprocessor hacks, one by one.

And then replacing it with even uglier stuff.
Of course that's a bit tongue-in-cheek, but point is, many languages with a C root that have tried getting rid of the preprocessor have come up with a bunch of stuff that, when it eventually gets to the point of being as flexible as the original preprocessor, becomes horribly convoluted.

[PlainName]

I like to do this for (embedded) configuration parsing.  Then, the hash table strings are configuration keys, and the values are pointers to the data, and the value-parsing function and its parameters in an union, so that although each configuration value is parsed using a single function, there only needs to be one per parameter type, instead of one per parameter.

I might be drifting towards this kind of thing. But probably I think I will more likely try each approach and see how they turn out.

[PlainName]

Unless you were going to call those functions extensively in time-constrained parts of the code, which I doubt here

If there is any constraint it would be towards code size. They would typically be called at init, and if a time-sensitive section needed repetitive access it would cache a copy.