Skip to Content

libipc

This is a page for the libipc educational project that we discussed in the course. The main target was to get people acquainted with writing actual C code.

I am not going to go over the design in detail here. Just the main line.

We will write a shared library that can be used to perform IPC (inter-process communication). The library should provide the user with simple and unified interface to perform several "types" of IPC. Such as file based, shared memory, network based..etc. The library should be plugin based, i.e. you can add/remove IPC methods with as little as code modification as possible and with the interface remaining the same.

This is basically the main requirement. I gave it a try and put the code on a public SVN repository to share the code with the other guys in the course. Comments and contributions are welcomed!

Comments

it doesn't compile [edited: it works :) ]

It doesn't compile here, does it compile on your machine walla el 3eib men 3andy?

edit: ok my bad, seems like i somehow download some old files with some new ones when you were updating them, that was yesterday

I still find it kinda weird that you included file_ipc.c in the tester and file_ipc.h in file_ipc.c.

and also shouldn't the user be the one who requests the method?

anyways, here's my code modification, I think (I don't remember exactly, that was about an hour or two ago) I only changed that 'file_ipc.c / file_ipc.h' thing.

Your 'todo'

Well...You made a todo in you cleaning function, and you said "figure out a way to know if the file is opened", I don't know about that but theoretically the file won't be opened cause you close the file after each operation so all you have to do here is unlink it. But, however, when more than one tester are being run, the function needs to check first if the one cleaning is in fact the last one using that key(file). So I think you can make a counter for each key(file opened) and when the counter reaches zero, you unlink.

And also you can 'malloc' the handle structure when requested and then 'free' it when cleaning. which means that pointers will be sent instead. I can make a patch with these fixes but just let me know if you have any comments about my first patch or are you sticking with the first code or what?

Michael behman

Did some more organization

Did some more organization in code. Added most of your patch..

The .c/.h thing was a n00b mistake..this actually is my first non-text book code to write. So, I am still trying to get my own style and get used to good practices.

I am sorry for the delay, I haven't had much time these days.

As for the cleaning function, you are right. What I was planing it to write an interface for the lib it self to manage several channels. And use a configuration file to manage several clients. I didn't do that yet, I am still including the code and not using it like a shared lib.

I don't think the use of a counter as a variable would work, since each process would have it's own memory space. So, I was thinking of using a configuration file as I said.

Why do you think using a malloc and free would be better? My understanding is that it's best to use dynamic allocation when I don't have an idea how much memory I would need. Which is not the case here..so it would be easier to use static allocation, right?

thanks a lot for the patch and for sharing your code :) I am trying to find time to read it now.

Please, if it's not much to ask, keep checking out my code and provide me with your comments and patches ;-)

thanks

MSameer's picture

malloc() if you don't know

malloc() if you don't know the size you want during compile time.

If you do know then allocate the variables on the stack.

// You leak if you don't free dst
void my_strcpy(char *dst, char *src) {
// No idea about the size of dst
assert(dst == NULL); // dst must be NULL

dst = (char *)malloc(strlen(src) + 1);

strcpy(dst, src);
}

Vs.

// You crash if you free buff
char *read5bytes(char *file) {
char buff[6]; // We will read 5 but fgets(); adds a terminating '\0'
FILE *foo = fopen(file, "r");
if (!foo) {
perror("fopen");
return NULL;
}

fgets(buff, 6, foo);

fclose(foo);

return buff;
}

Yeah you know, But

Yeah you do know the size of the variable(The handle structure) so you can put it on the stack if you want (also like what MSameer said) but my point here is, let's believe for a moment that we will use our IPCs in some big projects, in that case you might need to work with several keys, closing ones and opening others bla bla bla...so you don't want to leave all this unwanted memory in the stack, the solution is to malloc each handle(in the request handle function) and then free it when you are cleaning. Or at least that's what I understand so correct me if I'm wrong!

One other thing, Mayeb we need a third opinion because I'm not so sure about what I'm gonna say as this is also my first multi-files code and it's my first time to write a library, anyway, I was checking the new code and I noticed that you tend to put all the prototypes in the header file and then include it in the code, but from what I understand you should only put the prototypes to the functions you are going to use in the code that is including the library!

I know that sounds weird so tell me if you don't get what I mean :)

Oh yeah, by the way, that's what meant by a counter (a counter to be saved in the config file not a variable)

Michael Behman

MSameer's picture

I'm not sure I understand

I'm not sure I understand you but in general, header files should contain public API, structures, classes, macros, typedefs, ...

Everything that is needed in order to use the library or code.

Yeah

Yeah, that's exactly what I meant; Only what's public!

Michael behman

Good point regarding the

Good point regarding the dynamic memory allocation thing. We shouldn't drag all that memory around if we are going to build a big app.

For the multi-file thing. I intend to create a libipc.h file exposing the public API to the library user(that is what you guys are saying). This was the design we all agreed on in the session. I am trying to get it that way, and to build the thing to be an actual shared library. I am still a bit confused about the optimum organization, though..

Yeah but when you include

Yeah but when you include file_ipc.h in your code, the only function you use is "request handle" so you don't need to put the other functions' prototypes there, only this one. Plus, common.h should also be included to be able to use the handle structure, so instead of making the user(developer using the library) included it with file_ipc.h, you can include it in file_ipc.h, so that he includes the last one only. Which arises another question, by including common.h, you are allowing the user to use your error logging function, so is that ok?

All of the sudden, all of this doesn't seem like a big deal, I guess else than these things your code don't have any problems. Anyway, I'll take look at it again tomorrow.

So you never told me anything about my code, did you look at it? I know it sucks because it lacks commenting but just let me know if you had any suggestions.

By the way, I was thinking about adding a 'flush' function to my ipc, in case the user wanted to free the hard disk space(still the big projects thought). Anyway, I thought I'd share the idea with you in case you wanted to implement it too.

Michael behman

MSameer's picture

test_prog.c Why

  • test_prog.c
  1. Why calloc() ?
  2. I'm not sure what the prog does but it seems broken!
  3. Why do you use read(0, ...) to get the user input ?
  4. use:
void menu() {
while ((char in = getchar()) != EOF) {
switch (in) {
case '1':
// blah
break;

case '2':
// blah
break;

case '3':
return;

default:
// Unknown option!
break;
}
}

}
  1. Well, You are assuming not more than 100 bytes of input ?
  • I'm not going to check all of the code now but (Please comment it):
  1. If the preprocessor suff is accessed by your code only then move them into the .c file (NUMBER_OF_METHODS)
  2. Why is it defined twice ? Why is struct interface defined twice ?
  3. Where're the "include guards" ?
#ifndef __MY_HEADER_H__
#define __MY_HEADER_H__

#endif /* __MY_HEADER_H__ */

P.S. Why are you using multiple libraries ? ;-)

Calloc cause when I was

  • Calloc cause when I was using malloc, I found some garbage in the rest of the char array after the input.
  • Borken?? It's working here!! it had some problems and i fixed them and uploaded the new files a few mins before your comment.
  • well, actually I have no reason to use read to get the user input, maybe just cause I wanted to get used to the linux read/write functions (it's my first time to use them). I'll change it to get char.
  • well, about that '100 chars' assumption, I need some help about that. First reason, wanted to limit it because I was afraid of buffer overflow. Second reason, this is an ipc (I guess you noticed after all that checking), it reads and writes, so I wanted to define a way to know the end of each message. maybe I can read it character by character and stop by the '\0'.
  • NUMBER_OF_METHODS isn't used in the test_prog but i'm leaving in case the user wanted to check the number of methods there is in the current version
  • that struct interface, I think i fixed it in the new code
  • I'll try to implement the include guards, I've never used them before
  • about the multiple libraries, The design we agreed upon in the course is :

one interface library (to be included in the code). one library for each communication medium ( only the file IO till now). the if.h/if.c was a mistake when i started coding the ipc so I left it as it is.

I'll try to comment the code asap, i'm not a very good programmer so I need to get all the comments i can get to learn from them. thanks a lot!

Michael behman

Happy new year

MSameer's picture

I'm not telling you what

I'm not telling you what to do or the best approach to do stuff, I'm just giving random remarks since you've asked for an extra eye ;-)

Regarding calloc() and the garbage, why do you care as long as you have a terminating NULL '\0' byte at the end ? You can check memset() too...

I don't really understand what does the program do. I don't feel like reading all the uncommented code just to discover!

What I was proposing changes the whole function to use a loop instead of recursion. It allows one to press ^D ( = EOF) to quit too.

If you still want to keep the defines in the header then prefix them with LIBIPC for example but then, you will be defining them in the header and the .c file. I guess you should use one of them otherwise you will have problems if you forget to update one of them...

Include guards are there to prevent recursive inclusion of the same file:

Imagine foo.h which includes bar.h. The compiler will cry if the user includes both.

Imagine that foo.h includes bar.h and bar.h includes foo.h. Try it and see what will happen ;-)

Cheers,

Well...I was reading about

Well...I was reading about calloc and malloc + memset a few days ago, so as far as I can understand, the only problem with calloc is when it's allocating memory for a structure for a example and one of the members is a pointer. So I thought in my case it won't hurt. Plus, No, I don't have a terminating NULL because I'm still using read / write. (they don't work with terminating NULL, do they?)

What the program does is initialize the ipc handle and then ask the user whether he wants to read or write to it. So if two 'test_prog's were running at the same time, they'll be talking to each other (unless the key of one of them has changed)

I'll be working on the rest of the stuff this afternoon.

Michael behman

MSameer's picture

And please don't over

And please don't over engineer especially in the beginning :)

Cheers,

Another question that

Another question that popped onto my mind, in the "request handle" function, you send a handle structure and then return it. But if you look at the function carefully, you'll see that you should either send a pointer to the handle structure and work on it there, or you only call the function with the key and then return the handle structure generated. and if you are willing to use malloc, I suggest you go with the second and returned the pointer 'malloc'ed.

There was something else but I forgot it what it was.

Michael behman

Comment viewing options

Select your preferred way to display the comments and click "Save settings" to activate your changes.


Dr. Radut | book