This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
- From: Adhemerval Zanella <adhemerval dot zanella at linaro dot org>
- To: Lukasz Majewski <lukma at denx dot de>
- Cc: Joseph Myers <joseph at codesourcery dot com>, Paul Eggert <eggert at cs dot ucla dot edu>, Alistair Francis <alistair23 at gmail dot com>, Alistair Francis <alistair dot francis at wdc dot com>, GNU C Library <libc-alpha at sourceware dot org>, Siddhesh Poyarekar <siddhesh at gotplt dot org>, Florian Weimer <fweimer at redhat dot com>, Florian Weimer <fw at deneb dot enyo dot de>, Zack Weinberg <zackw at panix dot com>, Carlos O'Donell <carlos at redhat dot com>, Andreas Schwab <schwab at suse dot de>
- Date: Wed, 5 Feb 2020 13:11:18 -0300
- Subject: Re: [PATCH v3 3/7] y2038: alpha: Rename valid_timeval_to_timeval64 to valid_timeval32_to_timeval
- References: <20200129125914.11221-1-lukma@denx.de> <20200129125914.11221-3-lukma@denx.de> <6d790755-99c9-4aaa-9e04-cbea0b8f1f8f@linaro.org> <20200204235023.46d16ac6@jawa>
On 04/02/2020 19:50, Lukasz Majewski wrote:
> Hi Adhemerval,
>
>> On 29/01/2020 09:59, Lukasz Majewski wrote:
>>> Without this patch the naming convention for functions to convert
>>> struct timeval32 to struct timeval (which supports 64 bit time on
>>> Alpha) was a bit misleading. The name 'valid_timeval_to_timeval64'
>>> suggest conversion of struct timeval to struct __timeval64 (as in
>>> ./include/time.h).
>>>
>>> As on alpha the struct timeval supports 64 bit time it seems more
>>> readable to emphasis struct timeval32 in the conversion function
>>> name.
>>>
>>> Hence the helper function naming change to
>>> 'valid_timeval32_to_timeval'.
>>>
>>> Build tests:
>>> ./src/scripts/build-many-glibcs.py glibcs
>>>
>>> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
>>
>> Usually I don't see much gain in such changes. It the current name
>> clashing with a new proposed internal symbol?
>
> The problem here is that the name is a bit misleading, as alpha's
> struct timeval has 64 bit tv.sec (alpha generally supports 64 bit time).
>
> The rename here is to emphasis that we do convert "natural" (for alpha)
> struct timeval to struct timeval32 (which is needed when passing the
> pointer to syscalls).
>
> In that way the 'valid_timeval_to_timeval64' can follow the pattern,
> which we do have now in ./include/time.h.
>
> (it is just to make the naming convention more consistent)
I understand it and Iam not really against it in fact, it is usually I see
refactoring more profitable when it simplifies the resulting code by either
removing redundancy or adhering to newer code practices.