On Tue, Jul 05, 2022 at 12:57:45PM +0200, Omar Polo wrote:
> Stefan Sperling <stsp@stsp.name> wrote:
> > On Tue, Jul 05, 2022 at 10:52:22AM +0200, Omar Polo wrote:
> > > However, got-read-pack needs a big chunk of privsep.c anyway and it's a
> > > very frequently used one, so trimming down the size of privsep.c helps
> > > there too but not in a particularly significant way.
> > 
> > Maybe we could create a privsep_pack.c for all the stuff got-read-pack
> > needs and the other helpers do not?
> > 
> > That would already make privsep.c much smaller, and it will be trivial
> > to split it up more, e.g. per object type: privsep_blob.c,
> > privsep_tree.c, etc., and have a privsep_child.c for the forking code.
> 
> I gave it a spin for fun and turns out it's almost straightforward to
> do! :)
> 
> The diff is quite long (~7K) so instead of attaching it I'm linking my
> repo:
> https://git.omarpolo.com/?index_page=&path=got.git&action=summary&headref=split
> 
> (checkout the branch 'split')
> 
> It's not ready as-is still, privsep_misc.c is not a good name and
> there's a small hack in keeping two copies of static functions so it
> builds, but apart from that it seems to be working.  (gotweb included)

The direction is good. The less code we have in libexec helpers, the less
material there will be for ROP. And what you show here is that code
size reduction is possible just by splitting up privsep.c file into
smaller units, as predicted :)

Your privsep_misc.c contains some things that are probably exclusive to
got-read-pack, such as object enumeration. I would suggest to merge your
privsep_misc.c back into privsep.c for now. We can keep moving things
over one-by-one in future commits. There is no need to try to achieve
perfection in an initial commit of this. And the more code we are
moving at once, the higher are the chances that we'll break something.
Having "misc" in the filename also suggests that we haven't yet
identified suitable categories for grouping these functions together.