Re: [libvirt] [PATCH] cgroup: Free line even if no characters were read

On 07/16/2013 06:14 AM, Ján Tomko wrote: [1] good question; answer below (you didn't ask quite the right question, but it made me read the code - both the original code and this patch are broken; we need a v2). This VIR_FREE(line) is spurious, and should be deleted. The whole point of getline() is that it reuses a malloc'd buffer, possibly realloc()ing it to be larger, to minimize the malloc overhead. But by freeing every iteration of the loop, we've lost that advantage. This is bogus. If tmp is NULL, then len is extremely large, and will mess up the next iteration call to getline(). However, I could live with: if (tmp) { *tmp = '\0'; len = tmp - line; } which is slightly sub-optimal (it makes the next getline() call think it needs to call realloc(), when it really might already be large enough), but otherwise harmless (realloc() doesn't pay attention to *len, and is smart enough to be a no-op if the new size is no bigger than the existing real size). [1] To answer your question, STRPREFIX() is unsafe to call on NULL (it is a macro that boils down to strncmp, which must NOT have null arguments). But we are guaranteed that path and line are non-null here. This is another VIR_FREE that could be safely nuked, given the semantics of readline(). Yes, I agree that we need this, but we also need to fix the other bugs in the area. Looking forward to v2. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org