Re: [PATCH 10/13] libxt_rateest: avoid optional arguments

[Patrick McHardy <kaber@trash.net>]

On 24.05.2011 10:51, Jan Engelhardt wrote:
> 
> On Tuesday 2011-05-24 10:14, Patrick McHardy wrote:
>>>> On 23.05.2011 16:39, Jan Engelhardt wrote:
>>>>> Optional arguments make parsing unnecessarily harder - even more so
>>>>> than two-args. Right now, rateest even crashes because of it.
>>>>>
>>>>>  static const struct option rateest_opts[] = {
>>> [...]
>>>>> -	{.name = "rateest-bps1",  .has_arg = false, .val = OPT_RATEEST_BPS1},
>>>>> +	{.name = "rateest-bps1",  .has_arg = true, .val = OPT_RATEEST_BPS1},
>>> [...]
>>>>
>>>> This appears to be breaking backwards compatibility.
>>>
>>> Admittedly yes, though the fact that this has remained unseen for so
>>> long suggests that the potential user base is very small or not yet
>>> existing.
>>
>> I'm pretty sure this used to work at some point. Let me check history.
>>
>>> In my time with users in IRC, I notice that they in particular prefer
>>> hard stops in parsing over silent upgrades of rules[1], so as to
>>> actually become aware of the change upfront. As such, I believe the
>>> impact is well justified.
>>
>> Well, if it really was broken from the beginning I'm fine of course,
>> but I don't think that's case.
> 
> It works half of the time, and fails half of the time because it can
> run - since the beginning - into UB when using argv[optind].
> 
> Yes, the fix can be done in many ways, silent update is possible, but
> that is undesirable, as are optional arguments in the first place.
> 

Well, not breaking compatibility for the people it used to work for
certainly is desirable. If its not too much trouble, I very much
prefer a soft update and if necessary replacement of the old options
after some warning period.