wgtdkp / wgtcc

A small C11 compiler
MIT License
765 stars 130 forks source link

Reduce memory leaks #63

Closed luolent closed 3 years ago

luolent commented 3 years ago

Hello @wgtdkp This is a pull request to fix some of the memory leaks found in wtgcc, as reported by Valgrind. The fixes are quite simple and they follow your style. Even if the program frees all the memory after the program exits. I recommend this fix because it makes the code better and it can offer less troubles to someone else, who wants to use this project differently. Thanks!

codecov[bot] commented 3 years ago

Codecov Report

Merging #63 (e7fb79b) into master (c4cf781) will decrease coverage by 0.55%. The diff coverage is 85.18%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #63      +/-   ##
==========================================
- Coverage   85.38%   84.82%   -0.56%     
==========================================
  Files          23       23              
  Lines        5076     5088      +12     
==========================================
- Hits         4334     4316      -18     
- Misses        742      772      +30     
Impacted Files Coverage Δ
src/cpp.h 94.59% <ø> (-2.71%) :arrow_down:
src/cpp.cc 81.91% <84.00%> (-3.62%) :arrow_down:
src/main.cc 60.83% <100.00%> (+0.98%) :arrow_up:
src/evaluator.cc 50.79% <0.00%> (-5.56%) :arrow_down:
src/scanner.cc 93.18% <0.00%> (-0.33%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update c4cf781...e7fb79b. Read the comment docs.

wgtdkp commented 3 years ago

@luolent Thanks for your contribution!

Looks like the code coverage is broken, would you mind take a look? https://github.com/wgtdkp/wgtcc/pull/63/checks?check_run_id=1787092052

luolent commented 3 years ago

Thanks @wgtdkp! Sure, I can take a look at it. How would you like me to improve the code coverage? Should I commit additional tests?

wgtdkp commented 3 years ago

@luolent I think some of your added code are not covered by the travis tests, see the warning here https://github.com/wgtdkp/wgtcc/pull/63/checks?check_run_id=1787092052. But actually, the original implementation is not tested either :)

Let's have this merged, thanks for your contribution!