API design

[Jef Driesen]

I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a
header and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

/* device.c */

struct device {
/* some data here */
};

int
device_open (device **dev, const char *name)
{
if (dev == NULL)
return -1;

struct device *out = malloc (sizeof (struct device));
if (out == NULL)
return -1;

/* Do some stuff here */

*dev = out;
return 0;
}

As you can see, I made the internal data structure opaque by means of
the typedef and pointers to an incomplete data structure. Now my
questions are:

1. Is there any advantage to change the typedef to

typedef struct device *device;

and thus hiding the fact that the "device" type is actually a pointer.

2. I want to add support for a second type of device, which happens to
be very similar to the first one. For instance the read/write functions
are different, but the open/close functions and the contents of the
struct itself are the same. How can I implement this without copying the
code to a new pair of header and source files?

I was thinking to move the shared code to a common header and source
file and make the device-specific functions call the common functions.
But what should I do with the typdefs below, because both structs would
be the same in my case. But I would like to hide that from the user of
the library.

/* common.h (for internal use only) */

struct device {
/* some data here */
};

int device_open (device **dev, const char *name);

/* device1.h (public) */
typedef struct device1 device1;
int device1_open (device1 **dev, const char *name);
....

/* device2.h (public) */
typedef struct device2 device2;
int device1_open (device1 **dev, const char *name);
....

[Malcolm McLean]

"Jef Driesen" <je********@hotmail.com.invalidwrote in message

I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a
header and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

Take a book out of the standard library.

FILE *fp = fopen("filename.txt", "w");
if(!fp)
/* it failed */
fprintf(fp, "This file works\n");
fclose(fp);

Similarly

typedef struct device DEVICE;

DEVICE *dev = device_open("Fred");
if(!dev)
/* your device didn't open */
device_write(dev, "my name is Fred", 15);
device_close(dev);

--
Free games and programming goodies.
http://www.personal.leeds.ac.uk/~bgy1mm

[cr88192]

"Jef Driesen" <je********@hotmail.com.invalidwrote in message
news:wo***********************@phobos.telenet-ops.be...

I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a header
and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

device_open is ugly, IMO:
device *device_open (const char *name);
would be a little less ugly.

/* device.c */

struct device {
/* some data here */
};

int
device_open (device **dev, const char *name)
{
if (dev == NULL)
return -1;

struct device *out = malloc (sizeof (struct device));
if (out == NULL)
return -1;

/* Do some stuff here */

*dev = out;
return 0;
}

As you can see, I made the internal data structure opaque by means of the
typedef and pointers to an incomplete data structure. Now my questions
are:

1. Is there any advantage to change the typedef to

typedef struct device *device;

and thus hiding the fact that the "device" type is actually a pointer.

this may be acceptable (I actually do similar with a void pointer at one
point):
typedef void *dyt;

where dyt hides that it is a pointer.
this is a personal preference though.

in my case, 'dyt' is a dynamic type for a specific 'dynamic type' API of
mine...

2. I want to add support for a second type of device, which happens to be
very similar to the first one. For instance the read/write functions are
different, but the open/close functions and the contents of the struct
itself are the same. How can I implement this without copying the code to
a new pair of header and source files?

a typical approach is to make use of structs filled with function pointers.
these exist per-type, and are referenced by the specific open instances.

struct device_info_s {
int (*close)(device *dev);
int (*read)(device *dev, void *data, unsigned int size);
int (*write)(device *dev, const void *data, unsigned int size);
};

the frontend functions then dispatch through this struct.

int device_close (device *dev)
{
if(dev->info->close)
return(dev->info->close(dev));
return(-1);
}

this allows plugging lots of different subsystems into the same interface,
and is especially useful, for example, in things like custom VFS subsystems,
....

for example, I did this at one point for a VFS API I called 'VFILE', and,
apart from the fact that I rewrote it at one point (the internals, the
frontend API remained more or less unchanged), this API has remained in use
for at least around a decade (this is pretty much some of the oldest code in
my projects, and one of my first 'formalized' APIs).

from what I remember, I originally wrote it long ago for an RDBMS type
project (this was back when I was in middle school or so I think...). (damn,
middle school was a long time ago...).

I was thinking to move the shared code to a common header and source file
and make the device-specific functions call the common functions. But what
should I do with the typdefs below, because both structs would be the same
in my case. But I would like to hide that from the user of the library.

/* common.h (for internal use only) */

struct device {
/* some data here */
};

int device_open (device **dev, const char *name);

/* device1.h (public) */
typedef struct device1 device1;
int device1_open (device1 **dev, const char *name);
...

/* device2.h (public) */
typedef struct device2 device2;
int device1_open (device1 **dev, const char *name);
...

unnecessary API fragmentation is ugly, especially if you might expect more
of it in the future...

[Jef Driesen]

Malcolm McLean wrote:

"Jef Driesen" <je********@hotmail.com.invalidwrote in message

>I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a
header and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

Take a book out of the standard library.

FILE *fp = fopen("filename.txt", "w");
if(!fp)
/* it failed */
fprintf(fp, "This file works\n");
fclose(fp);

Similarly

typedef struct device DEVICE;

DEVICE *dev = device_open("Fred");
if(!dev)
/* your device didn't open */
device_write(dev, "my name is Fred", 15);
device_close(dev);

My question was not about the discussion to return the pointer with the
return value or with an output parameter. I did choose the last option
to be able to return an error code directly. This also makes the error
handling of the open function similar to the other functions.

[Roland Pibinger]

On Sat, 17 Nov 2007 21:13:40 +1000, "cr88192" wrote:

>"Jef Driesen" <je********@hotmail.com.invalidwrote in message

>I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a header
and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

device_open is ugly, IMO:
device *device_open (const char *name);
would be a little less ugly.

alternatively:

struct device_handle {
device* imp;
} device_handle;
typedef struct device_handle device_handle;

device_handle device_open (const char *name);

>2. I want to add support for a second type of device, which happens to be
very similar to the first one. For instance the read/write functions are
different, but the open/close functions and the contents of the struct
itself are the same. How can I implement this without copying the code to
a new pair of header and source files?

a typical approach is to make use of structs filled with function pointers.
these exist per-type, and are referenced by the specific open instances.

or just a 'switch on type' which invisible for the user, too.
--
Roland Pibinger
"The best software is simple, elegant, and full of drama" - Grady Booch

[Roland Pibinger]

On Sat, 17 Nov 2007 18:08:34 GMT, Roland Pibinger wrote:

>alternatively:

struct device_handle {
device* imp;
} device_handle;
typedef struct device_handle device_handle;

Should have been:

struct device_handle {
device* imp;
};
typedef struct device_handle device_handle;

[Chris Thomasson]

"cr88192" <cr*****@hotmail.comwrote in message
news:bb**************************@saipan.com...

>
"Jef Driesen" <je********@hotmail.com.invalidwrote in message

[...]

>2. I want to add support for a second type of device, which happens to be
very similar to the first one. For instance the read/write functions are
different, but the open/close functions and the contents of the struct
itself are the same. How can I implement this without copying the code to
a new pair of header and source files?

a typical approach is to make use of structs filled with function
pointers.
these exist per-type, and are referenced by the specific open instances.

struct device_info_s {
int (*close)(device *dev);
int (*read)(device *dev, void *data, unsigned int size);
int (*write)(device *dev, const void *data, unsigned int size);
};

the frontend functions then dispatch through this struct.

int device_close (device *dev)
{
if(dev->info->close)
return(dev->info->close(dev));
return(-1);
}

this allows plugging lots of different subsystems into the same interface,
and is especially useful, for example, in things like custom VFS
subsystems, ...

[...]

FWIW, here is a way to do minimalist interfaces in C:

http://groups.google.com/group/comp....106926ba5db19f

[cr88192]

"Chris Thomasson" <cr*****@comcast.netwrote in message
news:4r******************************@comcast.com. ..

"cr88192" <cr*****@hotmail.comwrote in message
news:bb**************************@saipan.com...

>>
"Jef Driesen" <je********@hotmail.com.invalidwrote in message

[...]

>>2. I want to add support for a second type of device, which happens to
be very similar to the first one. For instance the read/write functions
are different, but the open/close functions and the contents of the
struct itself are the same. How can I implement this without copying the
code to a new pair of header and source files?

a typical approach is to make use of structs filled with function
pointers.
these exist per-type, and are referenced by the specific open instances.

struct device_info_s {
int (*close)(device *dev);
int (*read)(device *dev, void *data, unsigned int size);
int (*write)(device *dev, const void *data, unsigned int size);
};

the frontend functions then dispatch through this struct.

int device_close (device *dev)
{
if(dev->info->close)
return(dev->info->close(dev));
return(-1);
}

this allows plugging lots of different subsystems into the same
interface, and is especially useful, for example, in things like custom
VFS subsystems, ...

[...]

FWIW, here is a way to do minimalist interfaces in C:

http://groups.google.com/group/comp....106926ba5db19f

yes.
this does not seem too much different from what I was suggesting here.
this is one way to do OO/abstract APIs.
actually, often in such APIs (my case), one can recognize that there are
often multiple paths to the same result, so, trying to use one method, if it
is not present, the wrapper may look into a few other possibilities
(fallback cases), before finally returning error.

an example in my 'VFILE' API (ok, actually for the most part a shameless rip
of stdio, as nearly everything is the same except for a 'V' or 'v' prefix on
everything, though in a few cases I used 'vf' as well, or used 'vf' as a
suffix...).

this was done so that it would be really easy to convert code to/from the
stdio file interface (though slightly limited, as this idea was developed
before I really discovered search-and-replace code-conversion, which would
mandate a slightly different naming convention to make work well, but
search-replace followed by more search-replace to deal with conversion
issues often works good enough...).

or such...

>

[Barry Schwarz]

On Sat, 17 Nov 2007 10:29:48 GMT, Jef Driesen
<je********@hotmail.com.invalidwrote:

>I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a
header and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

/* device.c */

struct device {
/* some data here */
};

int
device_open (device **dev, const char *name)
{
if (dev == NULL)
return -1;

struct device *out = malloc (sizeof (struct device));
if (out == NULL)
return -1;

/* Do some stuff here */

*dev = out;
return 0;
}

As you can see, I made the internal data structure opaque by means of
the typedef and pointers to an incomplete data structure. Now my
questions are:

1. Is there any advantage to change the typedef to

typedef struct device *device;

IMO, no advantage and a major disadvantage. If you make the change,
then code declaring a pointer will look like
device name;
which will inevitably at some point in the future lead some
unsuspecting maintenance programmer to believe he needs to add an &
when he passes name to some function expecting a pointer.

If you still decide to do it, change the typedef name from *device to
something like *device_ptr_t to give this unfortunate heir a hint.

>
and thus hiding the fact that the "device" type is actually a pointer.

Hide only the details that are worth hiding. As has already been
suggested, FILE is a precedent worth following.

>
2. I want to add support for a second type of device, which happens to
be very similar to the first one. For instance the read/write functions
are different, but the open/close functions and the contents of the
struct itself are the same. How can I implement this without copying the
code to a new pair of header and source files?

Again, FILE and the standard library show a reasonable approach. For
the things that are common, such as the struct declaration and the
open and close functions, you only need one. For the things that are
specific to a particular device (read and write functions), you need
unique versions. But the prototypes for these unique functions can
(not must) still be in a common header.

On my system, the standard headers include both the public and the
opaque information. You can't really hide the opaque data from the
programmer since he has to be able to read the headers to perform the
compile. It becomes a question of how much he has to wade through to
find the information.

If one program is capable of dealing with two device types, you have
the risk of a common structure associated with the second device being
passed to a function dedicated to the first device. Two typedef's
aliasing the same struct type will not solve this problem. If you
include in the structure a member indicating which of the possible
devices this instance of the structure applies to, your device
dependent routines can check before they do any real work.

>
I was thinking to move the shared code to a common header and source
file and make the device-specific functions call the common functions.

Putting code in a header is almost always a mistake. Maybe that's not
what you meant.

>But what should I do with the typdefs below, because both structs would
be the same in my case. But I would like to hide that from the user of
the library.

/* common.h (for internal use only) */

struct device {
/* some data here */
};

int device_open (device **dev, const char *name);

/* device1.h (public) */
typedef struct device1 device1;
int device1_open (device1 **dev, const char *name);
...

/* device2.h (public) */
typedef struct device2 device2;
int device1_open (device1 **dev, const char *name);
...

A single header: device.h

typedef struct device
{
...
enum type {device_1, device_2, ...};
...
} device;
device* device_open(...);
device* device_close(...);
int device_1_read(...);
...

A source file for each function, such as device_open.c

#include <device.h>
#include <stdlib.h>
device* device_open(...)
{
device *ptr = malloc(...);
if (ptr != NULL)
{
...
}
return ptr;
{

In device dependent functions, such as device_1_read

int device_1_read(device *ptr, ......)
{
if (ptr->type != device_1)
/*error handler*/
...
}

Whenever a device is opened

dev = device_open(...);
if (dev == NULL)
/* error handler */
dev_type = device_1; /*or _2 or whatever */
Remove del for email

[CBFalconer]

cr88192 wrote:

"Jef Driesen" <je********@hotmail.com.invalidwrote

>I'm writing a library (to communicate with a number of devices
over a serial port) and have some questions about the design. I
have now a header and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

device_open is ugly, IMO:
device *device_open (const char *name);
would be a little less ugly.

Only if the only error indication needed is failure vs success.

--
Chuck F (cbfalconer at maineline dot net)
<http://cbfalconer.home.att.net>
Try the download section.

--
Posted via a free Usenet account from http://www.teranews.com

[Barry Schwarz]

On Sat, 17 Nov 2007 18:08:34 GMT, rp*****@yahoo.com (Roland Pibinger)
wrote:

>On Sat, 17 Nov 2007 21:13:40 +1000, "cr88192" wrote:

>>"Jef Driesen" <je********@hotmail.com.invalidwrote in message

>>I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a header
and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

device_open is ugly, IMO:
device *device_open (const char *name);
would be a little less ugly.

alternatively:

struct device_handle {
device* imp;
} device_handle;

Why would you want a struct with only a single member? It just adds a
bunch of unneeded verbiage to both writing and reading the code.

<snip>
Remove del for email

[Richard]

Barry Schwarz <sc******@doezl.netwrites:

On Sat, 17 Nov 2007 18:08:34 GMT, rp*****@yahoo.com (Roland Pibinger)
wrote:

>>On Sat, 17 Nov 2007 21:13:40 +1000, "cr88192" wrote:

>>>"Jef Driesen" <je********@hotmail.com.invalidwrote in message
I'm writing a library (to communicate with a number of devices over a
serial port) and have some questions about the design. I have now a header
and source file like this:

/* device.h */
typedef struct device device;

int device_open (device **dev, const char *name);
int device_close (device *dev);
int device_read (device *dev, void *data, unsigned int size);
int device_write (device *dev, const void *data, unsigned int size);

device_open is ugly, IMO:
device *device_open (const char *name);
would be a little less ugly.

alternatively:

struct device_handle {
device* imp;
} device_handle;

Why would you want a struct with only a single member? It just adds a
bunch of unneeded verbiage to both writing and reading the code.

Plain common sense. Its either (a) stripped down for this example or (b) a
place holder for when the struct expands.

[Jef Driesen]

cr88192 wrote:

"Jef Driesen" <je********@hotmail.com.invalidwrote:

>2. I want to add support for a second type of device, which happens to be
very similar to the first one. For instance the read/write functions are
different, but the open/close functions and the contents of the struct
itself are the same. How can I implement this without copying the code to
a new pair of header and source files?

a typical approach is to make use of structs filled with function pointers.
these exist per-type, and are referenced by the specific open instances.

struct device_info_s {
int (*close)(device *dev);
int (*read)(device *dev, void *data, unsigned int size);
int (*write)(device *dev, const void *data, unsigned int size);
};

the frontend functions then dispatch through this struct.

int device_close (device *dev)
{
if(dev->info->close)
return(dev->info->close(dev));
return(-1);
}

this allows plugging lots of different subsystems into the same interface,
and is especially useful, for example, in things like custom VFS subsystems,
...

I think the solution you propose is useful if both devices share the
same interface, but with a different underlying implementation. And in
that case your solution allows to access both devices through a common
api. However in my situation, that is not entirely the case. Only some
functions of the devices are similar enough to be accessed through a
common api (like open/close/read/write), but not all functions on one
device have an equivalent on the other device.

So I do not necessary need a common interface. But there is some code
that is the same for both devices (like setting baudrate, timeouts,...)
and I want to re-use it, rather than duplicating it and thus needing to
maintain the same code it in two different locations.

>I was thinking to move the shared code to a common header and source file
and make the device-specific functions call the common functions. But what
should I do with the typdefs below, because both structs would be the same
in my case. But I would like to hide that from the user of the library.

/* common.h (for internal use only) */

struct device {
/* some data here */
};

int device_open (device **dev, const char *name);

/* device1.h (public) */
typedef struct device1 device1;
int device1_open (device1 **dev, const char *name);
...

/* device2.h (public) */
typedef struct device2 device2;
int device1_open (device1 **dev, const char *name);
...

unnecessary API fragmentation is ugly, especially if you might expect more
of it in the future...

What do you mean with "API fragmentation"?

[Roland Pibinger]

On Sun, 18 Nov 2007 10:06:35 -0800, Barry Schwarz wrote:

>On Sat, 17 Nov 2007 18:08:34 GMT, Roland Pibinger wrote:

>>struct device_handle {
device* imp;
};

Why would you want a struct with only a single member? It just adds a
bunch of unneeded verbiage to both writing and reading the code.

For more type safety and for making clear your intentions. You cannot
accidentally convert the handle to another type (pointers silently
convert from/to void*). Simple casts to another type don't work, too.
--
Roland Pibinger
"The best software is simple, elegant, and full of drama" - Grady Booch

[cr88192]

"Jef Driesen" <je********@hotmail.com.invalidwrote in message
news:He***********************@phobos.telenet-ops.be...

cr88192 wrote:

<snip>

>
I think the solution you propose is useful if both devices share the same
interface, but with a different underlying implementation. And in that
case your solution allows to access both devices through a common api.
However in my situation, that is not entirely the case. Only some
functions of the devices are similar enough to be accessed through a
common api (like open/close/read/write), but not all functions on one
device have an equivalent on the other device.

So I do not necessary need a common interface. But there is some code that
is the same for both devices (like setting baudrate, timeouts,...) and I
want to re-use it, rather than duplicating it and thus needing to maintain
the same code it in two different locations.

ok.

>>

unnecessary API fragmentation is ugly, especially if you might expect
more of it in the future...

What do you mean with "API fragmentation"?

this is where you end up with multiple incompatible APIs doing essentially
the same thing.

in non-trivial cases, this can make a horrible mess of things.

one has to keep track of all sorts of stuff, like different datatypes, API
functions, needing to implement "converters" to move data from one part of
the codebase to another, or even worse, to try to share the data and keep it
in sync, ...

IMO, it is far better to try to keep everything clean and unified.
as a codebase gets larger, this kind of thing can become very important, as,
say, changing some fundamental piece of the architecture, when you have,
say, several hundred kloc depending on it, can become very expensive (or, if
one is especially unlucky, break the entire project).

also, uniform APIs also make it a lot easier to reorganize things, as one
can (maybe even drastically) change the workings of one subsystem, while
still maintaining the internal integrity of others (wheras, with a much more
ad-hoc project, even minor changes can break things in an entirely different
subsystem).

so, not only should APIs be uniform, but they should also be opaque (aka: no
allowing "front end" code to dig around in the internals, instead it is
given a specific set of abstract types and functions, with each type and
function having a specific and well-defined task).

as such, we maintain a system of "invariants" that an API is held to, and
are free to do pretty much anything we want "under the hood" so long as
these basic rules are upheld...
so, it is better to try to adapt some semblance of "clean" practices early
on, rather than setting out to implement some horrible ad-hoc monstrosity...

it may make things slower, but IMO it pays off...
that is not to say though that sometimes fragmentation is not inevitable, or
some kind of holy grail above all other concerns, but it should at least be
tried for in general.