On 31.07.2023 21:03, Shawn Anastasio wrote:
> On 7/31/23 10:52 AM, Jan Beulich wrote:
>> On 28.07.2023 23:35, Shawn Anastasio wrote:
>>> Move the simple_strtoul routine which is used throughout the codebase
>>> from vsprintf.c to its own file in xen/lib.
>>>
>>> This allows libfdt to be built on ppc64 even though xen/common doesn't
>>> build yet.
>>>
>>> Signed-off-by: Shawn Anastasio <sanasta...@raptorengineering.com>
>>> ---
>>> xen/common/vsprintf.c | 37 -------------------------------------
>>> xen/lib/Makefile | 1 +
>>> xen/lib/simple_strtoul.c | 40 ++++++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 41 insertions(+), 37 deletions(-)
>>> create mode 100644 xen/lib/simple_strtoul.c
>>
>> What about its siblings? It'll be irritating to find one here and the
>> other there.
>
> I was debating whether to do this or not and ultimately decided to only
> make the minimum changes that were required right now. I can go ahead
> and make the change for its siblings as well.
>
>> Also please no underscores in (new) filenames unless there's a reason
>> for this. In the case here, though, I question the need for "simple"
>> in the file name in the first place.
>
> From a look at the other files in xen/lib there seemed to be a
> convention of naming files after the exact function they implement.
> Would you rather I rename it to just strtoul.c? Or simple-strotoul.c?
I'd prefer the former over the latter. While your observation of
convention has been true so far, I think the shorter names are to
be preferred here. They're not going to cause issues, as I don't
see us gaining non-simple_* functions.
>>> --- /dev/null
>>> +++ b/xen/lib/simple_strtoul.c
>>> @@ -0,0 +1,40 @@
>>> +/*
>>> + * Copyright (C) 1991, 1992 Linus Torvalds
>>> + */
>>> +
>>> +#include <xen/ctype.h>
>>> +
>>> +/**
>>> + * simple_strtoul - convert a string to an unsigned long
>>> + * @cp: The start of the string
>>> + * @endp: A pointer to the end of the parsed string will be placed here
>>> + * @base: The number base to use
>>> + */
>>> +unsigned long simple_strtoul(
>>> + const char *cp, const char **endp, unsigned int base)
>>> +{
>>> + unsigned long result = 0,value;
>>> +
>>> + if (!base) {
>>> + base = 10;
>>> + if (*cp == '0') {
>>> + base = 8;
>>> + cp++;
>>> + if ((toupper(*cp) == 'X') && isxdigit(cp[1])) {
>>> + cp++;
>>> + base = 16;
>>> + }
>>> + }
>>> + } else if (base == 16) {
>>> + if (cp[0] == '0' && toupper(cp[1]) == 'X')
>>> + cp += 2;
>>> + }
>>> + while (isxdigit(*cp) &&
>>> + (value = isdigit(*cp) ? *cp-'0' : toupper(*cp)-'A'+10) < base) {
>>> + result = result*base + value;
>>> + cp++;
>>> + }
>>> + if (endp)
>>> + *endp = cp;
>>> + return result;
>>> +}
>>
>> While moving, I think it would be nice if this stopped using neither
>> Xen nor Linux style. I'm not going to insist, but doing such style
>> adjustments right here would be quite nice.
>
> Especially if I'm going to be moving its siblings, I'd rather just copy
> the functions verbatim for this patch, if that's acceptable.
As said - I wouldn't insist, but in this case maybe I take the time
and make the slightly more "complete" change.
Jan
