On Fri, Jul 29, 2011 at 12:40:12AM +0200, Michel Hermier <[email protected]> wrote: > I need some people (vmiklos) to review the preliminary attached changes. > (Note they are not yet ment to be compiled as this).
So I'm just commenting your ideas, I can't really review a patch that
does not compile / not split it up to smaller, reviewable patches.
> In there you can see 3 axes of changes.
> - add.* remove.* sync.* trans.*: The first one that is functionnal and
> need still a bit more love (but that is currently enought for compiling)
> is a kind of transaction engine controled by structs. Instead of using a
> switch for each transaction, we use function from the struct. It helps to
> reduce the number of private exported symbols and make the libpacman a
> little bit more kernel style.
private exported symbols? I you want to reduce that, then just add
visibility markup and all will be gone. If you want to instruce such
structs with function pointers, I would rather refactor the db code:
that is really full of if (local) .. else <assume remote> code chunks,
and there such refactoring would *do* make the code more readable.
Note that in the db access case there are two users of that structs,
refactoring to use such a struct with a single user is unnecessary
complication, IMHO.
> - package.*: The second changes is a proposal to change of the package
> descrition that allows the definition of RODEPEND package variables to be
> exported. This extension as an intimate relation with the rodepend
> FrugalBuild but can be used to do a little bit more. This would help us to
> reduce the hidden dependencies introduced by the package plugin
> dependencies, using a new tricky variable I have to name. (This part is
> not functionnal, the current part only expose the description loader
> change required to load the modified files).
As I said on IRC, I don't think this makes too much sense (it just makes
building a little bit faster, zero benefit to end-users), but sure, as
long as you are willing to do the work, and fix packages in case
somewhere we depend on the fact that rodepends are currently installed
in the build chroot, why not. ;)
> - utils.*: Introduce _pacman_{free,malloc,zalloc} functions as generic
> memory helpers. the malloc one would reduce the code duplication for error
> handling, while the others are just syntax sugars to perform some usual
> memory stuff. (Yes I know we have the FREE macro, but it is usually not
> safe because we tends to forget that arguments are recomputed at each
> usage. Making the code hard to debug sometimes)
That's why C99 or somesuch introduced inline functions. But in case FREE
is about free(var); var = NULL, all an inline function can do is
inline_free(&ptr), and cluttering the code with a lot of "pointer to a
pointer" sounds ugly..
pgpLusaLFdVp8.pgp
Description: PGP signature
_______________________________________________ Frugalware-devel mailing list [email protected] http://frugalware.org/mailman/listinfo/frugalware-devel
