| Message ID | cover.1681117706.git.phillip.wood@dunelm.org.uk (mailing list archive) |
|---|---|
| Headers | show |
| Series | rebase: cleanup merge strategy option handling | expand |
On Mon, Apr 10, 2023 at 2:08 AM Phillip Wood <phillip.wood123@gmail.com> wrote: > > From: Phillip Wood <phillip.wood@dunelm.org.uk> > > Cleanup the handling of --strategy-option now that we no longer need to > support "--preserve-merges" and properly quote the argument when saving > it to disc. > > A hopefully final re-roll to fix a memory leak I spotted in V3. Doh, I clearly missed that leak too. > Changes since V3: > - Fixed a memory leak added in V3. The array returned by > split_cmdline() is allocated on the heap so we need to fee it. The > array elements come from the string passed to split_cmdline() so do > not need to be individually freed. And in particular, that string that stores those array elements will be freed by the strbuf_reset() that comes immediately after the read_strategy_opts() call (read_strategy_opts() is the sole caller of parse_strategy_opts()), and the fact that the string is freed is fine since your parse_strategy_opts() passes them to strvec_push() which will xstrdup() them and then own them. All that to say I agree with the fix.
From: Phillip Wood <phillip.wood@dunelm.org.uk> Cleanup the handling of --strategy-option now that we no longer need to support "--preserve-merges" and properly quote the argument when saving it to disc. A hopefully final re-roll to fix a memory leak I spotted in V3. Changes since V3: - Fixed a memory leak added in V3. The array returned by split_cmdline() is allocated on the heap so we need to fee it. The array elements come from the string passed to split_cmdline() so do not need to be individually freed. Changes since V2: - Added Elijah's Reviewed-by: trailer. - Fixed a typo in patch 4 spotted by Elijah. Changes since V1: I've rebased these patches onto 'master' to avoid conflicts with 'sg/parse-options-h-initializers' in the new patch 2 (this series depends on 'ab/fix-strategy-opts-parsing' but that has now been merged to master). Patch 1 - Unchanged. Patch 2 - New patch to store the merge strategy options in an "struct strvec". This patch also introduces a new macro OPT_STRVEC() to collect options into an "struct strvec". Patch 3 - Small simplification due to the changes in patch 2. Patch 4 - Moved the code to quote a list so it can split by split_cmdline() into a new function quote_cmdline() as suggested by Elijah. Patch 5 - Reworded the commit message as suggested by Elijah. Base-Commit: 140b9478dad5d19543c1cb4fd293ccec228f1240 Published-As: https://github.com/phillipwood/git/releases/tag/sequencer-merge-strategy-options%2Fv4 View-Changes-At: https://github.com/phillipwood/git/compare/140b9478d...7de1aa101 Fetch-It-Via: git fetch https://github.com/phillipwood/git sequencer-merge-strategy-options/v4 Phillip Wood (5): rebase: stop reading and writing unnecessary strategy state sequencer: use struct strvec to store merge strategy options rebase -m: cleanup --strategy-option handling rebase -m: fix serialization of strategy options rebase: remove a couple of redundant strategy tests alias.c | 18 +++++++ alias.h | 3 ++ builtin/rebase.c | 54 ++++----------------- builtin/revert.c | 20 ++------ parse-options-cb.c | 16 +++++++ parse-options.h | 10 ++++ sequencer.c | 58 ++++++++++------------ sequencer.h | 12 +++-- t/t3402-rebase-merge.sh | 21 -------- t/t3418-rebase-continue.sh | 88 +++++++++++++--------------------- t/t3436-rebase-more-options.sh | 18 ------- 11 files changed, 127 insertions(+), 191 deletions(-) Range-diff against v3: 1: 882b403423 = 1: 882b403423 rebase: stop reading and writing unnecessary strategy state 2: 1d8e59aa16 ! 2: 4b288e883d sequencer: use struct strvec to store merge strategy options @@ sequencer.c: void parse_strategy_opts(struct replay_opts *opts, char *raw_opts) - opts->xopts[i] = xstrdup(arg); + strvec_push(&opts->xopts, arg); } ++ free(argv); } + static void read_strategy_opts(struct replay_opts *opts, struct strbuf *buf) @@ sequencer.c: static void write_strategy_opts(struct replay_opts *opts) int i; struct strbuf buf = STRBUF_INIT; 3: e98ef5ce8c = 3: 4e040c1214 rebase -m: cleanup --strategy-option handling 4: a5e940e2d0 = 4: 671ee03503 rebase -m: fix serialization of strategy options 5: 2d1d328110 = 5: 7de1aa1016 rebase: remove a couple of redundant strategy tests