Hacker Newsnew | past | comments | ask | show | jobs | submitlogin
Show HN: Graph-based data storage REPL in C (github.com/incrediblesound)
39 points by jhedwards on May 27, 2015 | hide | past | favorite | 7 comments


Some general feedback:

In C you separate your implementation from your interface with header files. If you include .c files in .c files it will end up causing a lot of problems and start making includes order dependent (like how your code will break if you move #include "structures.c" down a few lines).

Generally you want a header file for each source file and put the stuff you want to share with other files in there (prototypes, typedefs, etc). For example, disk_utility.h would have prototypes for save_to_disk and read_from_disk and then instead of including disk_utility.c in your replay.c file you'd include disk_utility.h.

Without compiling the code I see a lot of things that are, at least, warnings. When you're learning you really want to compile with -Wall and pay attention to warnings.

It's a neat project and I hope you continue learning C, it's a great language.


Thanks for your feedback! I spent a lot of time chasing down memory leaks with this project, which was fun, but clearly that's not the last of the hurdles with C. My dream is to program space robots, which probably will never happen, but I feel one step closer every time I build something in C.


I do a lot of embedded C, so I generally avoid dynamically allocating memory, but when I do the way I try to avoid memory leaks is to always try to allocate and free in the same scope.

For example, you malloc in your get_line function but you free in main. I would probably do your line handling in main so the malloc/free are in the same scope.

Alternatively, I will sometimes make Create and Destroy functions that do the allocating/freeing but even when I do that I always try to keep the calls within the same scope so it's not easy to loose track of memory.

Practically allocating/freeing in the same scope is difficult so it takes a lot of effort to structure your code to accommodate.

If you're interested in mission-critical C programming you should check out the JPL C standards: http://lars-lab.jpl.nasa.gov/JPL_Coding_Standard_C.pdf

They're a little unrealistic for most non-space projects but it gives you a good idea of how safety-critical software is written in C.


As well as jotux's excellent reply, I'd like to add that clint, valgrind and friends help out a lot when starting out with C. Learning to use these tools well (by using them all the time), will teach you a lot about the language.

I also do a lot of embedded C, and can confirm that dynamic allocation is rare, so why not try restructuring your program to avoid it? It's good practice and will sharpen your skills by raising all sorts of questions like why dynamic allocation is discouraged in embedded systems in the first place, and what alternatives there are, and how to use a profiler to compare different trade-offs. Heaps of exploratory fun to be had!

And hey, Graph-Reply is really cool, and so are space robots.


I've made a pull request that contains a reasonable mount of feedback. I hope it's useful and instructive!


Those are great reading, thank you (even if I have nothing to do with the project :-). For others:

https://github.com/incrediblesound/Graph-Reply/commits?autho...


C is not a safe language sort of by definition, so you always have to consider all the possibilities. For example, you use sscanf in places, with fixed sized buffers. What happens if someone were to pass a lot of data into your program? What happens if the on-disk structure gets corrupted and a 'type' becomes more than 5 characters?




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: