On Thu, Jan 3, 2013 at 12:07 AM, <[email protected]> wrote:
> Good evening,
>
>
> I created a status bar with official link on suckless.org. I used
> gettemperature pattern with readline (from the page) and it works fine.
>
> Anyway, when I control memory leak with valgrind, 7 bytes are lost due to
> gettemperature (it increases every 3 seconds sleep). I tried to add free(),
> but it doesn't work. I don't understand.
>
> Maybe you could take a look.
>
>
eww, or, probably more to the point, welcome to c programming.
first of all, your smprintf function allocates heap memory and you're
using it in multiple levels, and worse, you're using multiple levels
of calls for a simple task at all. I get lost in it only from looking
at it. No wonter that leaves things lie dead. The part that gets lost
is - as your valgrind log clearly states - allocated by readfile()
line 104 and not handled anywhere in gettemperature() until 124.
Seriously, consider to use stack / decent stack-friendly data types -
that means: fixed-sized without having to malloc() and free() stuff -
where you don't want to mess with these system calls all the time. I'm
not sure why you don't rather have a function return a double by
reference there and have the char buffer[fixedsize] discarded in
function scope. fscanf() would probably not even ask that much.
--- /tmp/dwmstatus_leg.c 2013-01-03 06:32:37.163029095 +0100
+++ /tmp/dwmstatus_leg_new.c 2013-01-03 06:39:12.413038061 +0100
@@ -116,12 +116,14 @@
char *
gettemperature(char *base, char *sensor)
{
- char *co;
+ char *co, *ret;
co = readfile(base, sensor);
if (co == NULL)
return smprintf("");
- return smprintf(TEMPCOL, atof(co) / 1000);
+ ret = smprintf(TEMPCOL, atof(co) / 1000);
+ free(co);
+ return ret;
}
I still believe it is wrong to help you this way though, because you
should write things in a way you find them yourself, that's what C is
about.
The code is terrible as long as you don't know why things go awry.
After, it still may be bad, but you're in charge and find any
errors...
cheers!
mar77i
