On Mon, Oct 29, 2012 at 2:06 PM, Diego Novillo <dnovi...@google.com> wrote: > On Thu, Oct 25, 2012 at 6:39 PM, Lawrence Crowl <cr...@googlers.com> wrote: > >> The sbitmap non-bool returning bitwise operations have been merged with >> the bool versions. Sometimes this merge involved modifying the non-bool >> version to compute the bool value, and sometimes modifying bool version to >> add additional work from the non-bool version. The redundant routines have >> been ifdef'd out. I will remove the ifdef'd out code later. > > No #if 0 code, please. Let's just remove them. > >> The allocation functions have not been renamed, because we often do not >> have an argument on which to overload. > > Would it work if we made the first argument a reference to the bitmap > being allocated? > I suppose this shouldn't be a big deal.
It's definitely good to at least in one place see what kind of bitmap is actually being used ... ;) >> The cardinality functions have not >> been renamed, because they have different parameters, and are thus not >> interchangable. > > Why not change the parameters then? Agree, as a followup maybe. >> The iteration functions have not been renamed, because >> they are functionally different. > > Functionally different? How? It seems like the patch only goes skin > deep. I would rather make a true unification. If the only thing that > remains the different are the allocation functions, that's not a big > deal. But the point was to make everything else the same. Indeed. > I also notice that the patch does not rename any of the SET_BIT, > RESET_BIT functions in sbitmap.c. They should go - a followup is fine though. >> Tested on x86_64. Config testing in progress. > > You will also want to test on a couple other architecture on the farm. > By config testing, do you mean contrib/config-list.mk? Richard. > > Diego.
