On Tue, Jan 16, 2024 at 11:08 AM David Rowley <dgrowle...@gmail.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
