Hey Eric, On Fri, Jun 17, 2016 at 12:46 AM, Eric Sunshine <[email protected]> wrote: > On Thu, Jun 16, 2016 at 3:05 PM, Pranit Bauva <[email protected]> wrote: >> On Thu, Jun 16, 2016 at 2:44 AM, Eric Sunshine <[email protected]> >> wrote: >>> On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva <[email protected]> >>> wrote: >>>> Reimplement `is_expected_rev` & `check_expected_revs` shell function in >>>> C and add a `--check-expected-revs` subcommand to `git bisect--helper` to >>>> call it from git-bisect.sh . >>>> [...] >>>> Signed-off-by: Pranit Bauva <[email protected]> >>>> --- >>>> diff --git a/builtin/bisect--helper.c b/builtin/bisect--helper.c >>>> @@ -162,13 +162,44 @@ static int bisect_reset(const char *commit) >>>> +static int is_expected_rev(const char *expected_hex) >>>> +{ >>>> + struct strbuf actual_hex = STRBUF_INIT; >>>> + int res; >>>> + >>>> + if (strbuf_read_file(&actual_hex, git_path_bisect_expected_rev(), >>>> 0) < 0) { >>>> + strbuf_release(&actual_hex); >>>> + return 0; >>>> + } >>>> + >>>> + strbuf_trim(&actual_hex); >>>> + res = !strcmp(actual_hex.buf, expected_hex); >>>> + strbuf_release(&actual_hex); >>>> + return res; >>>> +} >>> >>> Not worth a re-roll, but this could be re-structured to avoid having >>> to remember to release the strbuf at all exits: >>> >>> struct strbuf actual_hex = ...; >>> int res = 0; >>> >>> if (strbuf_read_file(...) >= 0) { >>> strbuf_trim(...); >>> res = !strcmp(...); >>> } >>> strbuf_release(...); >>> return res; >>> >>> Alternately: >>> >>> if (strbuf_read_file(...) < 0) >>> goto done; >>> >>> strbuf_trim(...); >>> res = !strcmp(...); >>> >>> done: >>> strbuf_release(...); >>> return res; >>> >>> which is a bit less compact. >> >> I will avoid this for the reason that I will have to create a label >> for a lot of functions. If I choose to do this for one function, I >> think it would be more appropriate to do the same for other functions. >> There would be a lot of functions in future which would be in the same >> scenario and creating a separate label for each of them would be quite >> tedious. What do you think? > > Not sure what you're talking about. Label names are not shared across > functions. Anyhow, the first suggestion I presented above is more > concise than the 'goto' version.
Yes I am aware of the fact that labels aren't shared across functions. What I meant by "separate label" was that I will have to make a label "fail" in each function. But I recently noticed that its used quite a lot so I think it would be okay to use it. Will re-roll with using labels and goto. Regards, Pranit Bauva -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
