Re: [PATCH v2 6/6] bisect--helper: `bisect_write` shell function in C

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jun 16, 2016 at 9:01 PM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
> Hey Eric,
>
> On Fri, Jun 17, 2016 at 12:25 AM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
>> On Wed, Jun 15, 2016 at 10:00 AM, Pranit Bauva <pranit.bauva@xxxxxxxxx> wrote:
>>>
>>> Note: bisect_write() uses two variables namely TERM_GOOD and TERM_BAD
>>> from the global shell script thus we need to pass it to the subcommand
>>> using the arguments. After the whole conversion, we can remove the extra
>>> arguments and make the method use the two variables from the global scope
>>> within the C code.
>>
>> You could do this now rather than waiting for later. Instead of
>> passing these arguments to bisect_write(), create global variables in
>> this patch and assign them in the BISECT_WRITE case of
>> cmd_bisect__helper() before calling bisect_write().
>>
>> Not necessarily worth a re-roll, but would save you the effort of
>> having to explain it here and then change it in some later patch.
>
> I have actually done it in my next conversion which is converting
> check_and_set_terms()[1] which also sets those variables to some value
> so its more appropriate there.

My opinion about this is that using global variables would go against
a possible future libification of the bisect functionality and might
be less safe than just adding 2 parameters to a small number of
functions.

If we think that 2 parameters are too much or that there could be more
parameters to pass like this, we could just pass a pointer to a
'struct bisect_state' or something like that ;-)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]