chibicc icon indicating copy to clipboard operation
chibicc copied to clipboard

Memory leak

Open robinmoussu opened this issue 3 years ago • 5 comments

In a1ab0ff26f23c82f15180051204eeb6279747c9a, a memory leak is introduced. I added one possible way to fix it in 427fb998f44913460124f470d34edb2014108014. I just don't know how to create a PR targeting a given commit instead of a branch.

I am currently trying to re-write the whole compiler in Rust, one commit at a time (I will see if it was wise later!). I notice that (at least in the first commits), you C code isn't really robust against bad input, and would expose undefined behavior. Do you want me to create issues/PR against those?

robinmoussu avatar Nov 03 '20 16:11 robinmoussu

This project is just let memory leak. See Readme carefully.

gggin avatar Nov 05 '20 05:11 gggin

I was reading the commits one by one. The Readme doesn't exists yet!

But given that you plan to eventually publish a book on it, don't you plan to remove them?

robinmoussu avatar Nov 05 '20 08:11 robinmoussu

After reading the Readme, I understand your point of vue, but I think that you should comment it either in the commit message or in the code itself (or moving the Readme to an earlier commit).

robinmoussu avatar Nov 05 '20 08:11 robinmoussu

I'll update commit messages, and this should be addressed there. As to robustness, chibicc focuses only on handling correct code correctly and doesn't care too much about incorrect inputs. I think that's the appropriate prioritization, as oftentimes writing more code for error handling is a distraction especially during the very early stage of software development.

rui314 avatar Nov 12 '20 23:11 rui314

That's totally understandable. You should also write it clearly. And probably adding a comment on places where a check isn't done correctly. Trying to re-write the first commits in Rust made it really obvious that bad things where going on, even if at first glance the C looks fine. I absolutely agree that error handling would harm comprehension, but I think that adding a comment would help the reader understand why this code isn't production ready (and never will, it's not its goal).

robinmoussu avatar Nov 13 '20 08:11 robinmoussu