Re: [PATCH v15 08/27] bisect--helper: `is_expected_rev` & `check_expected_revs` shell function in C

Hi Pranit,

On 12/16/2016 08:00 PM, Pranit Bauva wrote:

On Wed, Dec 7, 2016 at 1:03 AM, Pranit Bauva [off-list ref] wrote:

quoted

quoted

I don't understand why the return value is int and not void. To avoid a
"return 0;" line when calling this function?

Initially I thought I would be using the return value but now I
realize that it is meaningless to do so. Using void seems better. :)

I just recollected when I was creating the next iteration of this
series that I will need that int.

quoted

quoted

quoted

+     case CHECK_EXPECTED_REVS:
+             return check_expected_revs(argv, argc);

See this.

This does not show that you need the "int", it just shows that you use
the return value of the function. But this return value is (in the
original shell code as well as in your v15) always 0. That is a sign
that the "void" return value makes more sense. Of course, then the line
above must be changed to

+     case CHECK_EXPECTED_REVS:
+             check_expected_revs(argv, argc);
+             return 0;

By the way, it also seems that the original function name
"check_expected_revs" was not the best choice (because the function
always returns 0, but the "check" implies that some semantically true or
false is returned).
On the other hand, it might also be useful (I cannot tell) to return
different values in check_expected_revs() — to signal the caller that
something changed or something did not change — but then ignore the
return value.

Best
  Stephan