Danh Doan <congdanhqx@xxxxxxxxx> writes:
> I think single line like:
>
> /* We accept 61st second for the single? leap second */
>
> Or something along the time, is good enough. Not sure if we want the
> word "single" there, though.
I do not particularly want to see the single but without it, the
single-one comment looks perfect.
>> > - if (num < 25 && num2 >= 0 && num2 < 60 && num3 >= 0 && num3 <= 60) {
>> > - tm->tm_hour = num;
>> > - tm->tm_min = num2;
>> > - tm->tm_sec = num3;
>> > + if (set_time(num, num2, num3, tm) == 0)
>> > break;
>> > - }
>> > return 0;
>>
>> This caller does become easier to follow, I would say. Nicely done.
>
> Yes, when I looked around date.c
>
> I saw that the only usecase for validate time is for setting it.
> And the incoming patch also has that usage.
>
> I chose to unify those code path to not buy me too much trouble.
>
> I'll take that "Nicely done" means this unification is OK for this
> series.
The OK was meant for this single place that was converted, not any
other place you'd use in the remainder of the series.
And I think it was not such a good idea to use it twice, but I think
with the suggested rewrite you took in v5, the other call site is
also OK.
Thanks.
