Re: Revise the Asserts added to bimapset manipulation functions

On Tue, Jan 16, 2024 at 11:08 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> I've now adjusted the patch to have all modifications to Bitmapsets
> return a newly allocated set. There are a few cases missing in master
> that need to be fixed (items 6-10 below):
>
> The patch now includes changes for the following:
>
> 1. Document what REALLOCATE_BITMAPSETS is for.
> 2. Provide a reusable function to check a set is valid and use it in
> cassert builds and use it to validate sets (Richard)
> 3. Provide a reusable function to copy a set and pfree the original
> and use that instead of duplicating that code all over bitmapset.c
> 4. Fix Assert duplication (Richard)
> 5. Make comments in bms_union, bms_intersect, bms_difference clear
> that a new set is allocated. (This has annoyed me for a while)
> 6. Fix failure to duplicate the set in bms_add_members() when b == NULL.
> 7. Fix failure to duplicate the set in bms_add_range() when upper < lower
> 8. Fix failure to duplicate the set in bms_add_range() when the set
> has enough words already.
> 9. Fix failure to duplicate the set in bms_del_members() when b == NULL
> 10. Fix failure to duplicate the set in bms_join() when a == NULL and
> also fix the b == NULL case too
> 11. Fix hazard in bms_add_members(), bms_int_members(),
> bms_del_members() and bms_join(), where the code added in 7d58f2342
> could crash if a == b due to the REALLOCATE_BITMAPSETS code pfreeing
> 'a'. This happens in knapsack.c:93, although I propose we adjust that
> code now that empty sets are always represented as NULL.

Thank you so much for all the work you have put into making this patch
perfect. I reviewed through the v3 patch and I do not have further
comments. I think it's in good shape now.

Thanks
Richard