Martin Ågren <martin.agren@xxxxxxxxx> writes: > The strings allocated in `setup_unpack_trees_porcelain()` are never > freed. Provide a function `clear_unpack_trees_porcelain()` to do so and > call it where we use `setup_unpack_trees_porcelain()`. The only > non-trivial user is `unpack_trees_start()`, where we should place the > new call in `unpack_trees_finish()`. > > The `opts` string array contains multiple copies of the same pointers. > Be careful to only free each pointer once, then zeroize the whole array > so that we do not leave any dangling pointers. The verb to make it zero or fill it with zero is "to zero", I would think. To be honest I am not sure if I like the way this change is done. The clear_unpack_trees_porcelain() function has too intimate knowledge of what happens inside the setup_unpack_trees_porcelain() function; it not just knows which fields are always allocated but which are duplicates, which must be double checked for updates whenever the latter gets modified, yet there is no large warning sign painted in red in the latter, so it is easy to change the latter and invalidate the assumption the former makes by mistake, leading to new leaks and/or double freeing. I wonder if an approach that is longer-term a bit more maintainable is to add a new string-list instance to opts, save these xstrfmt()'ed messages to it when setup_unpack_trees_porcelain() create them, and then make clear_unpack_trees_porcelain() pay *no* attention to msg[] array and the positions of these allocated messages and duplicates but just reclaim the resources held in that string-list, or something like that.
