Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code generation API names #2725

Closed
kodebach opened this issue May 26, 2019 · 7 comments

Comments

2 participants
@kodebach
Copy link
Contributor

commented May 26, 2019

The code generation API mainly uses the following macros (there are other ways to use it, but this is the intended way):

#define elektraGet(elektra, tag) ELEKTRA_GET (tag) (elektra)

#define elektraGetV(elektra, tag, ...) ELEKTRA_GET (tag) (elektra, __VA_ARGS__)

#define elektraGet2(elektra, result, tag) ELEKTRA_GET (tag) (elektra, result)

#define elektraGet2V(elektra, result, tag, ...) ELEKTRA_GET (tag) (elektra, result, __VA_ARGS__)

#define elektraSet(elektra, tag, value, error) ELEKTRA_SET (tag) (elektra, value, error)

#define elektraSetV(elektra, tag, value, error, ...) ELEKTRA_SET (tag) (elektra, value, __VA_ARGS__, error)

#define elektraSize(elektra, tag) ELEKTRA_SIZE (tag) (elektra)

#define elektraSizeV(elektra, tag, ...) ELEKTRA_SIZE (tag) (elektra, __VA_ARGS__)
  • The Get, Set and Size (array size) parts of the names should be obvious.
  • The V suffix indicates versions that take additional arguments (needed for replacing # and _ in keynames). This is needed, because __VA_ARGS__ cannot be empty (if someone knows a way around that, that would be nice).
  • The Get2 macros are used for non-allocating structs, i.e. structs that don't use malloc in the getter and instead use a result pointer.

Get2 is definitely not perfect, but I couldn't come up with short alternative. Suggestions are welcome.


Note: Internally (e.g. ELEKTRA_GET_OUT_PTR_SIGNATURE) OutPtr is used to indicate the result pointer. That might be an alternative, but to me that makes it seem like I could use elektraGet or elektraGetOutPtr depending on which I prefer, which is not the case.

@kodebach kodebach added this to must have for release in lcdproc May 26, 2019

@markus2330

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

The V suffix indicates versions that take additional arguments

V is more or less standard (vprintf, ...). But to be consistent with ksVNew, we should consider to name it elektraVGet.

(needed for replacing # and _ in keynames)

Please elaborate.

This is needed, because VA_ARGS cannot be empty

Afaik this is not possible (in a portable way). This is why we also have the F variants of warnings/errors.

The Get2 macros are used for non-allocating structs, i.e. structs that don't use malloc in the getter and instead use a result pointer.

Why do we need this? keyGetString (a similar API) is basically unused.

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

(needed for replacing # and _ in keynames)

Please elaborate.

The code generation works by creating a static inline function for each Key in the specification. The use of the elektraGet macro resolves to this function. If a specified key is inside an array # or below a wildcards _, the generated function expects arguments for these key parts (an array index, or a string to replace _). The __VA_ARGS__ part of the macro is used for these arguments.

Why do we need this? keyGetString (a similar API) is basically unused.

keyGetString is very different IMO. First of all, the alternative keyString returns a const char *, meaning it doesn't allocate any additional memory that has to be freed. Secondly for strings we have strdup, if we need to make a separate copy of the string, which is much easier to use than keyGetString.

elektraGet2 serves a completely different purpose and is only used for structs. The normal signature for elektraGet means that the value of the requested key has to be returned. For structs this is only possible by using malloc. With elektraGet2, however, it becomes possible to use a struct on the stack, which doesn't have to be freed.

If the struct is specified as non-allocating (i.e. to be used with elektraGet2) the main use of the struct becomes avoiding multiple elektraGet calls (less code, single conversion from string per key), and having an object that can easily be passed around in code that contains the configuration.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented May 26, 2019

If a specified key is inside an array # or below a wildcards _

very nice, please add an example in the readme/tutorial

Is there any protection against passing wrong parameters? (E.g. in ksNew there is via
ELEKTRA_SENTINEL)

elektraGet2 serves a completely different purpose and is only used for structs.

Why not call it elektraGetStruct?

Btw. "V" might not fit after all: V usually indicates that a va_list is passed, which is here not the case.

Btw2. How is now the compatibility of the API? Is it C99 (even with code generation)?

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented May 26, 2019

Is there any protection against passing wrong parameters?

The macros are actually completely statically typed. If you take a look at the generated code (+ elektra.h) and go through what the preprocessor would do, you will see that the preprocessor resolves the macro to a simply function call with all the correct parameters.

Here is an example function from one of the test cases:

/**
* Get the value of 'person/_/children/#'.
*
* @param elektra Instance of Elektra. Create with loadConfiguration().
succeed_if_same_string ($1)
*
* @return the value of 'person/_/children/#', free with ELEKTRA_STRUCT_FREE (StructPerson).
*///
static inline Person * ELEKTRA_GET (PersonChildren) (Elektra * elektra ,
const char * name1 ,
kdb_long_long_t index1
)
{
char * name = elektraFormat ("person/%s/children/%*.*s%lld", name1 ,
elektra_len (index1), elektra_len (index1), "#___________________", (long long) index1 );
const char * actualName = elektraFindReference (elektra, name);
elektraFree (name);
if (actualName == NULL || strlen (actualName) == 0)
{
return NULL;
}
return ELEKTRA_GET (StructPerson) (elektra, actualName);
}

Why not call it elektraGetStruct?

Also thought about that, but if the struct is using malloc e.g. because you have arrays inside the struct, or reference to other structs (like in lcdexec for example), you need to use the normal elektraGet. So elektraGetStruct would be really confusing.

Is it C99 (even with code generation)?

AFAIK it should be C99 compatible (the tree and econf examples use -std=c99). It definitely is not C89 (initialization in for loops), but it may be valid in the GNU89 extension.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented May 27, 2019

resolves the macro to a simply function call with all the correct parameters.

very good

Also thought about that, but if the struct is using malloc e.g. because you have arrays inside the struct, or reference to other structs (like in lcdexec for example), you need to use the normal elektraGet. So elektraGetStruct would be really confusing.

What about elektraGetAlloca? Or elektraGetPointer? Or also elektraGetFillStruct? Or even elektraGetStringStructElement (similar to ArrayElement)?

AFAIK it should be C99 compatible (the tree and econf examples use -std=c99).

very good

@kodebach

This comment has been minimized.

Copy link
Contributor Author

commented Jun 7, 2019

I decided to rename elektraGet2 to elektraFillStruct. I also decided to keep the V suffix, since anything more fitting (e.g. WithArgs) would be much longer and the elektraGet* macro uses are long enough already because of the tag names.

In the end it doesn't really matter, because using elektraGet (without suffix) with a tag that needs arguments, will result in a compiler error anyway.

@markus2330

This comment has been minimized.

Copy link
Contributor

commented Jun 9, 2019

elektraFillStruct

Yes, good choice.

V suffix, since anything more fitting (e.g. WithArgs)

I would prefer WithArgs but I understand that renaming is a lot of effort.

In the end it doesn't really matter

Good names always matter, even if you get errors if you pick the wrong one.

@kodebach kodebach closed this Jun 20, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.