| Message ID | eaffac30-8bd0-6018-5186-ca53d1becfe5@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen: beginnings of moving library-like code into an archive | expand |
Hi Jan, On 23/10/2020 11:17, Jan Beulich wrote: > ... into its own CU, to build it into an archive. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > > ... into its own CU, to build it into an archive. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> Acked-by: Julien Grall <jgrall@amazon.com> Cheers,
On 18/11/2020 17:39, Julien Grall wrote: > Hi Jan, > > On 23/10/2020 11:17, Jan Beulich wrote: >> ... into its own CU, to build it into an archive. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> I forgot to mention the commit message was duplicated. >> ... into its own CU, to build it into an archive. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> > > Acked-by: Julien Grall <jgrall@amazon.com> > > Cheers, >
On 23/10/2020 11:17, Jan Beulich wrote: > ... into its own CU, to build it into an archive. > > Signed-off-by: Jan Beulich <jbeulich@suse.com> > --- > xen/common/lib.c | 39 ---------------------------------- > xen/lib/Makefile | 1 + > xen/lib/parse-size.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ > 3 files changed, 51 insertions(+), 39 deletions(-) > create mode 100644 xen/lib/parse-size.c What is the point of turning this into a library? It isn't a leaf function (calls simple_strtoull()) and doesn't have any any plausible way of losing all its callers in various configurations (given its direct use by the cmdline parsing logic). ~Andrew
On 24.11.2020 01:58, Andrew Cooper wrote: > On 23/10/2020 11:17, Jan Beulich wrote: >> ... into its own CU, to build it into an archive. >> >> Signed-off-by: Jan Beulich <jbeulich@suse.com> >> --- >> xen/common/lib.c | 39 ---------------------------------- >> xen/lib/Makefile | 1 + >> xen/lib/parse-size.c | 50 ++++++++++++++++++++++++++++++++++++++++++++ >> 3 files changed, 51 insertions(+), 39 deletions(-) >> create mode 100644 xen/lib/parse-size.c > > What is the point of turning this into a library? It isn't a leaf > function (calls simple_strtoull()) and doesn't have any any plausible > way of losing all its callers in various configurations (given its > direct use by the cmdline parsing logic). It's still a library function. As said earlier, I think _all_ of what's now in lib.c should move to lib/. That's how it should have been from the beginning, or stuff shouldn't have been put in lib.c. The one alternative I see is to move the code next to parse_bool() / parse_boolean(), in kernel.c, or put all parse_*() into a new common/parse.c. Jan
diff --git a/xen/common/lib.c b/xen/common/lib.c index a224efa8f6e8..6cfa332142a5 100644 --- a/xen/common/lib.c +++ b/xen/common/lib.c @@ -423,45 +423,6 @@ uint64_t muldiv64(uint64_t a, uint32_t b, uint32_t c) #endif } -unsigned long long parse_size_and_unit(const char *s, const char **ps) -{ - unsigned long long ret; - const char *s1; - - ret = simple_strtoull(s, &s1, 0); - - switch ( *s1 ) - { - case 'T': case 't': - ret <<= 10; - /* fallthrough */ - case 'G': case 'g': - ret <<= 10; - /* fallthrough */ - case 'M': case 'm': - ret <<= 10; - /* fallthrough */ - case 'K': case 'k': - ret <<= 10; - /* fallthrough */ - case 'B': case 'b': - s1++; - break; - case '%': - if ( ps ) - break; - /* fallthrough */ - default: - ret <<= 10; /* default to kB */ - break; - } - - if ( ps != NULL ) - *ps = s1; - - return ret; -} - typedef void (*ctor_func_t)(void); extern const ctor_func_t __ctors_start[], __ctors_end[]; diff --git a/xen/lib/Makefile b/xen/lib/Makefile index 764f3624b5f9..99f857540c99 100644 --- a/xen/lib/Makefile +++ b/xen/lib/Makefile @@ -2,3 +2,4 @@ obj-$(CONFIG_X86) += x86/ lib-y += ctype.o lib-y += list-sort.o +lib-y += parse-size.o diff --git a/xen/lib/parse-size.c b/xen/lib/parse-size.c new file mode 100644 index 000000000000..ec980cadfff3 --- /dev/null +++ b/xen/lib/parse-size.c @@ -0,0 +1,50 @@ +#include <xen/lib.h> + +unsigned long long parse_size_and_unit(const char *s, const char **ps) +{ + unsigned long long ret; + const char *s1; + + ret = simple_strtoull(s, &s1, 0); + + switch ( *s1 ) + { + case 'T': case 't': + ret <<= 10; + /* fallthrough */ + case 'G': case 'g': + ret <<= 10; + /* fallthrough */ + case 'M': case 'm': + ret <<= 10; + /* fallthrough */ + case 'K': case 'k': + ret <<= 10; + /* fallthrough */ + case 'B': case 'b': + s1++; + break; + case '%': + if ( ps ) + break; + /* fallthrough */ + default: + ret <<= 10; /* default to kB */ + break; + } + + if ( ps != NULL ) + *ps = s1; + + return ret; +} + +/* + * Local variables: + * mode: C + * c-file-style: "BSD" + * c-basic-offset: 4 + * tab-width: 4 + * indent-tabs-mode: nil + * End: + */