View Single Post
Posts: 1,808 | Thanked: 4,272 times | Joined on Feb 2011 @ Germany
#23
@Mentalist Traceur,

Just some off-line debugging.. maybe my comments don't solve anything, but as a "past" C programmer there are some things that hurt my eyes.

Code:
struct cal *cal_s;
..
if(cal_init(&cal_s) < 0 ..)
Before calling cal_init() I would first initialize cal_s to NULL. We don't know what libcal does exactly, so better to be on the safe side.

Then, before doing cal_read_block make sure that cal_s is not NULL. Otherwise cal_read_block will most likely segfault.

Same with cal_finish(). What if cal_finish expects a non-NULL pointer but cal_read_block actually handles a NULL pointer by merely returning a negative value? I bet 10$ that cal_finish does free() at some point, and we all know what happens (or what should happen anyway) when you free a NULL pointer.

Now, AND and OR. Obviously we don't know what libcal is doing, but it is supposedly writing something to tmp *and* returning a length, presumably of the data being written to tmp, meaning tmp may not necessarily be NUL-terminated (i.e. not a standard C-type string), meaning strcpy() might freak out and write more than 128 bytes to rd_mode_current, meaning that .. well, I don't have to explain that do I?

So, to be on the safe side, use strncpy making sure that you write at most 128 bytes (or sizeof(rd_mode_current)/sizeof(char)) and preferably len, i.e. min(len, 128).
Then put a zero at the end just for security. rd_mode_current[min(len, 128)] = '\0'.

And before that (AND/OR), I would still do if(len < 1 || (tmp == NULL)), as you probably would only continue if len > 0 and tmp is not NULL.

Namely, if for any reason (did I say we don't know what libcal does?) len > 0 and tmp is NULL strcpy() will also segfault.

That's it for now. I'd suggest you add a lot of printf()'s to aid in debugging. Or use assert() wherever you assume something.

e.g. before doing strcpy(a, "hello") put assert(a != NULL). This way you will see the truck before it hits you
 

The Following 3 Users Say Thank You to reinob For This Useful Post: