Using void pointers in generic accessor function with ANSI C

I have a working example of a piece of C code that I'm using to teach myself about using pointers effectively in a non-trivial application. (I have a dream to contribute a missing feature to a C library which I'm relying on.)

My sample code loo like this:

#include <stdio.h>
#include <stdlib.h>

struct config_struct {
  int port;
  char *hostname;
};

typedef struct config_struct config;

void setup(config*);
void change(config*);
void set_hostname(config*, char*);
void get_hostname_into(config*, char**);
void teardown(config*);
void inspect(config*);

int main() {

  char* hostname;
  config* c;
  c = calloc( 1, sizeof(config));

  setup(c);
  inspect(c);

  change(c);
  inspect(c);

  set_hostname(c, "test.com");
  inspect(c);

  get_hostname_into(c, &hostname);
  inspect(c);  
  printf("retrieved hostname is %s (%p)\n", hostname, &hostname);

  teardown(c);
  printf("retrieved hostname is %s (%p) (after teardown)\n", hostname, &hostname);

  return EXIT_SUCCESS;
}

void setup(config* c) {
  c->port     = 9933;
  c->hostname = "localhost";
}

void change(config* c) {
  c->port     = 12345;
  c->hostname = "example.com";
}

void set_hostname(config* c, char* new_hostname) {
  c->hostname = new_hostname;
}

void get_hostname_into(config* c, char** where) {
  *where = c->hostname;
}

void teardown(config* c) {
  free(c);
}

void inspect(config* c) {
  printf("c is at %p\n", c);
  printf("c is %ld bytes\n", sizeof(*c));
  printf("c:port is %d (%p)\n", c->port, &(c->port));
  printf("c:hostname is %s (%p)\n", c->hostname, &开发者_C百科(c->port));
}

It's required by the nature of the library (the function is get_session_property(session*, enum Property, void*) - thus I'm looking for a way to dereference a void pointer; I was able to successfully implement this for an int, but have been kicking my heels trying to figure out how to do it for a char* (something about a void* to int making some sense, but I can't fathom how to do it for void* to char*.

My successful implementation (with tests) for the library is on my Github fork of the project, here.

The closest I have come is:

enum Property { Port, Hostname };
void get_property(config*, enum Property, void*);
void get_property(config* c, enum Property p, void* target) {
  switch(p) {
    case Port:
      {
        int *port;
        port = (int *) target;
        *port = c->port;
      }
      break;
    case Hostname:
      {
        char *hostname;
        hostname = (char *) target;
        *hostname = c->hostname;
      }
      break;
  }
}

Which mercifully doesn't segfault, but also leaves char *get_hostname_into_here null, raising the warning (which I can't figure out:)

untitled: In function ‘get_property’:
untitled:33: warning: assignment makes integer from pointer without a cast

Full source code of my contrived example here; please when answering explain, or recommend any reading you have on using void pointers and/or good C style, it seems like everyone has a different idea, and a couple of people I know in the real world simply said "the library is doing it wrong, don't use void pointers) - whilst it would be nice if the library would make the struct public; for encapsulation and other good reasons, I think the void pointers, generic function approach is perfectly reasonable in this case.

So, what am I doing wrong in my hostname branch of the get_property() function that the char* is NULL after the call to get_property(c, Hostname, &get_hostname_into_here);

char *get_hostname_into_here;
get_property(c, Hostname, &get_hostname_into_here);
printf("genericly retrieved hostname is %s (%p)\n", get_hostname_into_here, &get_hostname_into_here);
// Expect get_hostname_into_here not to be NULL, but it is.

full source code for example (with output).

---

Like I said in my comments above, it's not possible to give a precise answer, because it's not clear what the aim is here. But I see two possibilities:

1: target is pointing at a char buffer

If this is the case, then it would seem that you'll need to copy the contents of your string into that buffer. This is not possible to do safely, because you don't know how big the receiving buffer is. But if you don't care about that, then you need to do something like:

strcpy((char *)target, c->hostname);

2: target is pointing at a char *

If this is the case, then the intention is presumably either to modify that char * to point at the existing string, or to dynamically create a new buffer, copy the string, and then modify the char * to point at it.

So either:

char **p = (char **)target;
*p = c->hostname; 

or:

char **p = (char **)target;
*p = malloc(strlen(c->hostname)+1);
strcpy(p, c->hostname);

Note

You get the warning message because in this line:

*hostname = c->hostname;

*hostname is of type char, whereas c->hostname is of type char *. The compiler is telling you that this conversion doesn't make any sense. If I were you, I would set your compiler up to treat warnings as errors (e.g. with the -Werror flag for GCC), because warnings should always be adhered to!

---

The get_property function should be altered so that target is a double void pointer, meaning that you can change the pointer itself (not only the memory it refers to):

void get_property (config *c, enum Property p, void **target) {
  switch (p) {
    case Port:
      *((int *) (*target)) = c->port;
      break;

    case Hostname:
      *target = c->config;
      break;
  }
}

And then use the function like that:

int port;
int *pport = &port;
char *hostname;
get_propery(c, Port, &pport);
get_propery(c, Hostname, &hostname);

---

There's no answer to your question until you provide more details about the get_property function. It is clear that the void *target parameter is used to pass an external "space" in which you are supposed to place the result - the value of the requested property.

What is the nature of that recipient space?

In case of a int property it it pretty clear form your code: the pointer points to some int object in which you are supposed to place the property value. Which is what you do correctly.

But what about string properties? There are at least two possibilities here

1) The void *target parameter points to the beginning of a char [] buffer, which is supposedly large enough to receive any property value. In that case your code should looks as follows

case Hostname:
  {
    char *hostname = target;
    strcpy(hostname, c->hostname);
  }
  break;

The function in this case would be called as

char hostname_buffer[1024];
get_property(c, Hostname, hostname_buffer);

This is actually the "correct" way to do it, except that you need to take certain steps to make sure you don't overrun the target buffer by some long property value.

2) The void *target parameter points to an pointer of char * type, which is supposed to receive the hostname pointer value from the property. (In that case the target actually holds a char ** value.) The code would look as

case Hostname:
  {
    char **hostname = target;
    *target = c->hostname;
  }
  break;

The function in this case would be called as

char *hostname;
get_property(c, Hostname, &hostname);

This second variant doesn't look good to me, since in this case you are essentially returning a pointer to internal data of property structure. It is not a good idea to give the outside world access to the internals of [supposedly opaque] data structure.

P.S. One generally does not need to explicitly cast to and from void * pointrs in C language.

---

get_hostname_into_here is defined as:

char *get_hostname_into_here;

And you're passing a reference to it, namely a char**. In get_property, you're casting the void* into a char* instead of a char**, and then dereferencing it before the assignment. In order to get the string correctly, use:

case Hostname:
{
   char **hostname;
   hostname = (char **) target;
   *hostname = c->hostname;
}
break;

---

Let's consider a simplified example of your get_property function which is the same in all the important ways:

void get_property_hostname(config* c, void* target) {
    char * hostname = (char *) target;
    *hostname = c->hostname;
}

On the first line of the function, you are making a "char *" pointer which points to the same location as "target". On the second line of the function, when you write *hostname = ..., you are writing to the char that hostname points at, so you are writing to the first byte of memory that target points to. This is not what you want; you are only giving one byte of data to the caller of the function. Also, the compiler complains because the the left-hand side of the assignment has type "char" while the right-hand side has the type "char *".

There are at least three correct ways to return a string in C:

1) Return a pointer to the original string

If you do this, the user will have access to the string and could modify it if he wanted to. You must tell him not to do that. Putting the const qualifier on it will help achieve that.

const char * get_property_hostname(config* c) {
    return c->hostname;
}

2) Duplicate the string and return a pointer to the duplicate

If you do this, the caller of the function must pass the duplicate string to free() when he is done using it. See the documentation of strdup.

const char * get_property_hostname(config * c) {
    return strdup(c->hostname);
}

3) Write the string to a buffer that the caller has allocated

If you do this, then it is up to the caller of the function when and how he wants to allocate and free the memory. This is what a lot of APIs in the Microsoft Windows operating system do because it offers the most flexibility to the caller of the function.

void get_property_hostname(config * c, char * buffer, int buffer_size)
{
    if (strlen(c->hostname)+1 > buffer_size)
    {
        // Avoid buffer overflows and return the empty string.
        buffer[0] = 0;
    }
    else
    {
        strcpy(buffer, c->hostname);
    }
}

Then to use this function, you can do something like:

void foo(){
    char buffer[512];
    get_property_hostname(c, buffer, sizeof(buffer));
    ...
    // buffer is on stack, so it gets freed automatically when foo returns
}

EDIT 1: I will leave it as an exercise for you to figure out how to integrate the ideas presented here back into your generic get_property function. If you take the time to understand what is going on here, it shouldn't be too hard, but you may have to add some extra parameters.

EDIT 2: Here's how you would adapt method 1 to use a void pointer that points to a char * instead of using a return value:

void get_property_hostname(config* c, void * target) {
    *(char **)target = c->hostname;
}

Then you would call it like this:

void foobar() {
    char * name;
    get_property_hostname(c, &name);
    ...
}