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 <[email protected]> > > > > > +#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
