Decide which .h files are public and move private to /src/arch

[GoogleCodeExporter]

The reason to have an /include folder is to only export the header files
that are public available functions. 

We don't want people to be using the csp_malloc, csp_queue_enqueue etc.
functions, they should be using freertos/posix directly. (Or do we? In this
case CSP also becomes an OS wrapper which i don't think is so cool :)

As a consequence some of the headers in /include should be moved under
/src/arch/ (because they interface to the functionality implemented in this
folder alone), and be references by the source files like

#include "arch/csp_queue.h"
and not like:
#include <csp/csp_queue.h>

This is critical to get sorted before writing documentation.

Original issue reported on code.google.com by johan.de...@gmail.com on 1 May 2010 at 12:40

[GoogleCodeExporter]

csp.h still needs the type definitions of e.g csp_queue_handle_t and
csp_bin_sem_handle_t, so maybe we should merge the #ifdef sections of 
csp_queue.h,
csp_semaphore.h, etc. into csp.h and just move the function prototypes into 
arch/ ?
That would leave only csp.h in include/csp. Would that be a possible solution?

Original comment by j...@satlab.org on 1 May 2010 at 7:05

[GoogleCodeExporter]

I'm thinking something entirely different.

Why not hide the contents of those data structures for the user.

typedef csp_conn_t void *;
typedef csp_socket_t void *;

csp_socket_t * csp_socket();
csp_conn_t * csp_accept(csp_socket_t * socket, int timeout);

This then does not need any definition of csp_queue_handle_t and stuff. And at 
the
same time, we ensure that the user will never try to read or change anything in 
the
data-structures, leading to more security.

Okay, there is one time where a user would like to acces conn->idind for 
example to
read port number in task_server for example. We should then implement a 
function like
this:

csp_id_t * csp_conn_get_hdr(csp_conn_t * conn);

Which could be a simple inliner implemented in csp_conn.c

Otherwise i don't se any problems in this do you?

The files inside the CSP implementation that need to access the data-structure 
will
include another file which redeclares the typedef to the structure. (I hope 
this is
possible)

Original comment by johan.de...@gmail.com on 1 May 2010 at 7:27

[GoogleCodeExporter]

In fact i just found out some more here:
http://xinutec.org/~pippijn/en/programming_data-hiding.xhtml

We can forward declare the csp_conn_t structure and use it as an opaque pointer 
like
this:

in csp.h

struct csp_conn_s; // Forward declaration
typedef csp_conn_s * csp_conn_t;

csp_conn_t csp_accept(...);

in csp_conn.h

struct csp_conn_s {
  int a;
  int b;
  int c;
};

Now all files including only csp.h will have access to the abstract version of
csp_conn_t, but files also including csp_conn.h will have access to the full 
version
and be able to dereference a csp_conn_t. (i hope :)

Original comment by johan.de...@gmail.com on 1 May 2010 at 7:46

[GoogleCodeExporter]

Berkeley sockets hides the internal structs from the user by referencing the 
socket
with an integer similar to a file descriptor. Our csp_socket_t and csp_conn_t 
could
maybe be merged and referenced by an integer instead. This would also mean that 
a
separate csp_close and csp_socket_close functions are not needed. 

Original comment by j...@satlab.org on 1 May 2010 at 7:47

[GoogleCodeExporter]

Yes okay, for the connections the integer abstraction implementation is very 
simple
and nice, since the integer could just be the index in the static connection
structure array! - The only problem is that functions like csp_send now needs 
to call
a function in csp_conn.c in order to convert the integer to a structure pointer,
because the connection array is a static declaration. So i don't know.

Sockets (which are dynamically allocated) i still think the best solution is a 
void
pointer abstraction. (Since this is in fact what we point to in reality). This 
will
also make it easy to implement a socket free at a later time.

I'm currently writing a small piece of example code to see if it will work.

Original comment by johan.de...@gmail.com on 1 May 2010 at 8:05

[GoogleCodeExporter]

Okay, got this working:

public.h:
---------

struct conn_s;
typedef struct conn_s * conn;

conn csp_accept(void);
void csp_print_a(conn);

private.h
---------

struct conn_s {
        int a;
};

implementation.c
----------------

#include "public.h"
#include "private.h"
static struct conn_s conn_private;

conn csp_accept(void) {
        conn_private.a = 1;
        return &conn_private;
}

void csp_print_a(conn conn_p) {
        printf("A is %u\r\n", conn_p->a);
}

main.c
------

#include "public.h"

void main(void) {
        conn conn_user_copy = csp_accept();
        csp_print_a(conn_user_copy);
}

The output of this programme is: A is 1.

If i try the following in main:

void main(void) {
        conn conn_user_copy = csp_accept();
        conn_user_copy->a = 2;
        csp_print_a(conn_user_copy);
}

The program fails to compile with:

error: dereferencing pointer to incomplete type

Which is totally correct. - We don't want people to be dereferencing our 
pointers :)

Original comment by johan.de...@gmail.com on 1 May 2010 at 8:16

[GoogleCodeExporter]

It's even simpler than i thought:

struct conn_s; <-- This line is NOT needed!
typedef struct conn_s * conn;

So instead of:

typedef struct {
    csp_queue_handle_t conn_queue;
} csp_socket_t;

We just write:

typedef struct csp_socket_s csp_socket_t;

And define 

struct csp_socket_s {
   csp_queue_handle_t conn_queue;
}

in a private header which will be placed somewhere in /src/. Which can contain
compile-time define's or even just use different include path based on compiler 
setting.

Problem solved :)

Original comment by johan.de...@gmail.com on 1 May 2010 at 8:29

[GoogleCodeExporter]

Ja, okay fandt lige den definitive forklaring:
http://en.wikipedia.org/wiki/Opaque_pointer

Tror bare at det er det vi gør :)
Eller hvad siger du?

Original comment by johan.de...@gmail.com on 1 May 2010 at 9:26

[GoogleCodeExporter]

Enig, det virker som en god løsning. Jeg kan godt flytte rundt på det i 
aften, men
jeg har lige lidt grafteori der skal overståes først...

Original comment by j...@satlab.org on 2 May 2010 at 7:20

[GoogleCodeExporter]

This issue was closed by revision r15.

Original comment by johan.de...@gmail.com on 3 May 2010 at 1:57

• Changed state: Fixed

[GoogleCodeExporter]

No, it's just a partial fix! - Don't write "Fixed issue 11" in a comment if you 
don't
mean it :)

Original comment by johan.de...@gmail.com on 3 May 2010 at 1:58

• Changed state: Accepted

[GoogleCodeExporter]

r24 is another partial fix to this issue.

Original comment by j...@satlab.org on 7 May 2010 at 7:23

[GoogleCodeExporter]

r24 fixed this.

Original comment by j...@satlab.org on 7 May 2010 at 11:57

• Changed state: Fixed