On Sat, Oct 17, 2015 at 07:18:25PM +0200, Pierre Neidhardt wrote:
On 15-10-17 12:56:57, Dave Reisner wrote:
On Sat, Oct 17, 2015 at 06:49:14PM +0200, Pierre Neidhardt wrote:
On 15-10-17 09:11:02, Dave Reisner wrote:
On Sat, Oct 17, 2015 at 12:04:11PM +0200, Pierre Neidhardt wrote:
Signed-off-by: Pierre Neidhardt <ambrevar@gmail.com> +#define TITLE_MAXLEN 64
Surely, you examined the POT files to determine this value. Could you leave behind a comment explaining as much so that it's clear what this relates to for future readers?
Actually I did! :) Longest title string is less than 26 characters long. But you are right, I will add a comment in the next patch.
This raises a good question however: what would be a good maximum value for titles? (Titles longer than TITLE_MAXLEN get truncated.)
Well, it's a bit of a misfeature, IMO. Have you run a benchmark to determine that heap allocating the right amount is substantially slower or more complicated than keeping it on the stack? After all, this code is only run once, so I'm a little skeptical that we gain anything in exchange to bearing the maintenance cost of this magic value.
Agreed on performance, it's complitely negligible (2*TITLE_COUNT allocs).
But the matter is more to cleanup here. Since `dump_pkg_full` can be called
Also not terribly concerning. It's a fixed size leak which the operating system cleans up for us on process exit. This isn't library code, but I realize this also probably violates pacman "style" and touches a lot of people's OCDs in funny ways.
several times, we cannot free `titles` from here. It has to be done from one of the callers. Which means that 'titles' should be stored somewhere else, and thus passed to `make_aligned_titles` down the callstack. If we do not want to make it fully global, that being said.
Solutions:
* Stay static. * Go global. * Pass `tables` down the callstack.
This third option has the distinct advantage that you can now reasonably *test* your code.
Staying static is simple and efficient IMO. TITLE_MAXLEN can be set high enough that it will never get rechead.
-- Pierre Neidhardt
The major advances in civilization are processes that all but wreck the societies in which they occur. -- A.N. Whitehead