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.