[pacman-dev] [PATCH 3/3] Align titles automatically in information display

Pierre Neidhardt ambrevar at gmail.com
Sat Oct 17 17:18:25 UTC 2015


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 at 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
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.

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


More information about the pacman-dev mailing list