Skip to content

c-bindings: adios2_variable_type_string

Created by: germasch

I'm not using the C interface, nor am I planning to ever do that, but I wanted to point out that IMHO adios2_variable_type_string should be changed while (or if) it's still possible. (same for attribute, too)

/**
 * Retrieve variable type in string form "char", "unsigned long", etc.
 * @param type output, string form "int", "char"
 * @param size output, type size without '\0'
 * @param variable handler
 * @return adios2_error 0: success, see enum adios2_error for errors
 */
adios2_error adios2_variable_type_string(char *type, size_t *size,
                                         const adios2_variable *variable);

In my opinion, C bindings should try to follow the conventions of the C language in as far as possible, but this does not.

  • This function cannot be safely used, since one has to pass an allocated char * to it, but one will not know how many characters are needed until after the function returns.
  • Okay, so one can uses char type[100] and that's probably safe forever (but even in scientific software, buffer overflow bugs should be avoided if possible).
  • If one then goes to print the returned string, one will get trailing garbage, because it's not 0-terminated.

Semi-safe use would go like this:

char type[100];
size_t size;
adios2_error rc = adios2_variable_type_string(type, &size, myVar);
assert(rc == 0);
assert(size < 99);
type[size] = 0;
printf("type: %s\n", type);

I think chances that a user gets that right are pretty slim. It's a C API, so it's not going to be pretty, but my suggestion would be:

adios2_error adios2_variable_type_string(const char **p_type,
                                         const adios2_variable *variable);

Use:

const char* type;
adios2_error rc = adios2_variable_type_string(&type, myVar);
assert(rc == 0);
printf("type: %s\n", type);

My suggestion would be to return a statically allocated const char*, so if the user wants, they can strdup() it, but in general, one wouldn't need to, and not worry about freeing the string.

Obviously, this is an API change, though at least it will be known at compile time due to the changed function signature. It looks like this function is never used at all nor tested within ADIOS2, but obviously there could be external users. I'd be happy to provide to create a patch if this is considered a useful change.