unalignedmemoryaccess
unalignedmemoryaccess

Reputation: 7441

Is one type with 2 definitions undefined behavior in C?

Consider a library where you have some code. For example let's do some point X and Y manipulation.

And then you build your library where you don't want to allow users to access to your struct variable, so far I'm using this approach and it seems to work ok.

lib.h:

#ifndef __LIB_H
#define __LIB_H

#ifdef __LIB_INTERNAL
//Structure for single point
typedef struct {
    int x, y;
} Point;

//Casted pointer
#define _P(in)      ((Point *)(in))
#endif

//Define pointer for public use as void pointer
typedef void* Point_p;

//Create point
Point_p createPoint(int x, int y);

#endif

lib.c:

//Define LIB_INTERNAL to allow visible access
#define __LIB_INTERNAL
#include "lib.h"
#include "stdlib.h"

Point_p createPoint(int x, int y) {
    Point_p p = malloc(sizeof(Point));
    _P(p)->x = x; //_P is visible in this function
    _P(p)->y = y;
    return p;
}

main.c:

#include "lib.h"

int main() {
    Point_p p = createPoint(1, 2); //OK
    Point *ptr = createPoint(1, 2); //Error as Point is not visible public

    p->x = 4; //Error as Point_p is void *
}

This way I'm making sure that user don't have direct access to Point variable and it is forced to use functions to perform operations on this point.


Now I'm thinking of another approach. But first, sizeof(void *) and sizeof(Point *) is always the same so I would like to use this approach by showing Point_p to lib.c as typedef Point* Point_p and to all other files which are not part of library as typedef void* Point_p.

lib.h

#ifndef __LIB_H
#define __LIB_H

#ifdef __LIB_INTERNAL
//Structure for single point
typedef struct {
    int x, y;
} Point;

//Define pointer for private use as Point pointer
typedef Point* Point_p;

#else

//Define pointer for public use as void pointer
typedef void* Point_p;

#endif


//Create point
Point_p createPoint(int x, int y);

#endif

lib.c:

//Define LIB_INTERNAL to allow visible access
#define __LIB_INTERNAL
#include "lib.h"
#include "stdlib.h"

Point_p createPoint(int x, int y) {
    Point_p p = malloc(sizeof(Point));
    p->x = x; //_P is not needed because Point_p is visible as Point *
    p->y = y;
    return p;
}

main.c: the same as previous


Question

Is this undefined behavior? Because in second approach, lib.c sees Point_p as Point *, but main.c still sees it as void * and therefore lib.c has access to members directly without casting before and main.c does not have it neither can cast because Point structure is hidden.

Upvotes: 22

Views: 1271

Answers (2)

Petr Skocik
Petr Skocik

Reputation: 60087

Yes, it is. Struct pointers are not guaranteed to have the same representation as void pointers.

However, all struct pointers are guaranteed to have the same representation regardless of tag,

6.2.5p28:

... All pointers to structure types shall have the same representation and alignment requirements as each other. All pointers to union types shall have the same representation and alignment requirements as each other. ...

so the common, well-defined way to solve this is to only provide a forward declaration of a struct in the public header and then use pointers to that.

public_header.h

struct Point; //the private header provides the full definition
struct Point* createPoint(int x, int y);
//...

private_header:

#include "public_header.h"
struct Point { int x, y; }; //full definition

That approach also doesn't suffer from the type-looseness of void pointers.

(You should also avoid using identifiers starting with two underscores or an underscore and an uppercase letter as well as filescope identifiers/tags that start with an underscore (don't ever start identifiers with an underscore if you want to keep it simple)—that's undefined behavior too (See 7.1.3 Reserved Identifiers)).

Upvotes: 25

John Bollinger
John Bollinger

Reputation: 180518

I'm using this approach so far and it works just ok without any undefined behaviour

I suppose you mean that the code you present exhibits the observable behavior you expect under the circumstances in which you have tested it, which is quite a different thing from being without undefined behavior. Certainly, the code as you originally posted it absolutely had undefined behavior as a result of applying the sizeof operator to an expression of type void.

But first, sizeof(void *) and sizeof(Point *) is always the same

C does not guarantee that, nor that the representations of those pointer types are equivalent. You can, however, safely convert a Point * to a void * and back, where "safely" means that the result will compare equal to the original Point *.

I would like to use this approach by showing Point_p to lib.c as typedef Point* Point_p and to all other files which are not part of library as typedef void* Point_p.

This is not safe and formally would exhibit undefined behavior, which might or might not manifest in a way that you notice. Even though you can convert between them, Point * and void * are not "compatible" types in the standard's sense of the term.

A better pattern for implementing opaque types in C is to use incomplete types. That would look something like this:

lib.h:

// User header for lib
#ifndef __LIB_H
#define __LIB_H

// Structure for a single point -- NO BODY DECLARED
typedef struct point Point;

// Create point
Point *createPoint(int x, int y);

#endif

lib.c:

#include <stdlib.h>
#include "lib.h"

// complete the definition of struct point
struct point {
    int x, y;
};

Point *createPoint(int x, int y) {
    Point *p = malloc(sizeof(*p));
    p->x = x;
    p->y = y;
    return p;
}

With that, you don't have any messy macros controlling what parts of the header should be used, and you don't even have to worry about client code just declaring __LIB_INTERNAL to get access to the structure members, because they aren't in the header at all. This all nevertheless has perfectly well-defined behavior, and, moreover, better type safety than does using void * for everything.

Upvotes: 11

Related Questions