Re: [PATCH]: sbitmap changes to accomodate new bitmapimplementation.

[Daniel Berlin <dan at cgsoftware dot com>]

Andreas Jaeger <aj@suse.de> writes:

> Daniel Berlin <dan@cgsoftware.com> writes:
> 
>> Andreas Jaeger <aj@suse.de> writes:
>> 
>>  > Daniel Berlin <dan@cgsoftware.com> writes:
>>> 
> ...]
>>> [...]
>>>> *************** void
>>>> *** 96,102 ****
>>>>   sbitmap_copy (dst, src)
>>>>        sbitmap dst, src;
>>>>   {
>>>> !   memcpy (dst->elms, src->elms, sizeof (SBITMAP_ELT_TYPE) * dst->size);
>>>>   }
>>>>   
>>>>   /* Determine if a == b.  */
>>>> --- 96,128 ----
>>>>   sbitmap_copy (dst, src)
>>>>        sbitmap dst, src;
>>>>   {
>>>> !   unsigned int bitstocopy, remainder, setsize;
>>>> !   /* Trivial case */
>>>> !   if (dst == src)
>>>> !     return;
>>>> !   bitstocopy = src->n_bits;
>>>> !   /* If we are copying from a bigger to a smaller, only copy the
>>>> !      smaller number of bits */
>>> 
>>> But in this case we lose some bits - is this really wanted and
>>> correct? 
>> Yes, it is.
>>>  Shouldn't we abort here?
>> No, because that would leave someone with no way to do exactly this,
>> without resorting to EXECUTE_IF_SET_IN_SBITMAP or something.
>> One would assume that if you are deliberately trying to copy from a
>> larger to a smaller, that you want to truncate the result.
>> This is different than logical operations, where doing such a thing
>> doesn't make sense at all (Or'ing two equal sized sbitmaps into a
>> smaller one is never what someone wants to do.  It's also not a well
>> defined operation, unlike copying.)
>> However, this is all from personal experience working on the bitmap
>> implementation, which is the only thing that works with multiple sizes
>> of sbitmaps.  Since nothing else does it, i chose a behavior that
>> seemed defined, reasonable, and was useful to the bitmap
>> implementation.
>> 
>> If you are really concerned, i could make sbitmap_copy return an int,
>> specifying whether the bitmap was truncated or not as a result of the
>> copy.
>> Then, people who don't want this could do
>> 
>> if (!sbitmap_copy (a, b))
>>    abort();
> 
> I leave this to others to decide.  I'm not sure what's the best way to
> define this interface, it just puzzles me.
Well, i could also leave sbitmap_copy as is (modifying it to abort if
the sbitmaps are different sizes), and add sbitmap_copy_n to
copy a certain number of bits, only aborting if the number of bits you
ask to copy is larger than the destination.

Does that make more sense?

> 
> Thanks for your other comments,
> Andreas
> -- 
>  Andreas Jaeger
>   SuSE Labs aj@suse.de
>    private aj@arthur.inka.de
>     http://www.suse.de/~aj

-- 
"My dental hygienist is cute.  Every time I visit, I eat a whole
package of Oreo cookies while waiting in the lobby.  Sometimes
she has to cancel the rest of the afternoon's appointments.
"-Steven Wright