From mboxrd@z Thu Jan 1 00:00:00 1970 From: Heinrich Schuchardt Date: Wed, 08 Apr 2020 04:28:52 +0000 Subject: [PATCH v4 02/31] lib: Add a function to convert a string to upper case In-Reply-To: <4A84FC6A-D9FD-4ACF-A922-26353E1F4970@gmx.de> References: <20200408030107.168474-1-sjg@chromium.org> <20200407210023.v4.2.I1ca01985ee4914667e7a2ba006de71c2546eada3@changeid> <4A84FC6A-D9FD-4ACF-A922-26353E1F4970@gmx.de> Message-ID: <18F5D670-A753-4BF5-84F8-DDCB2F993F6A@gmx.de> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Am April 8, 2020 4:18:12 AM UTC schrieb Heinrich Schuchardt : >Am April 8, 2020 3:00:38 AM UTC schrieb Simon Glass : >>Add a helper function for this operation. Update the strtoul() tests >to >>check upper case as well. >> >>Update FAT writing to use this new function. >> >>Signed-off-by: Simon Glass >>--- >> >>Changes in v4: >>- Add a new patch to convert a string to upper case >> >>Changes in v3: None >>Changes in v2: None >> >> fs/fat/fat_write.c | 13 ++------- >> include/vsprintf.h | 12 ++++++++ >> lib/strto.c | 8 +++++ >> test/str_ut.c | 73 >++++++++++++++++++++++++++++++++++------------ >> 4 files changed, 77 insertions(+), 29 deletions(-) >> >>diff --git a/fs/fat/fat_write.c b/fs/fat/fat_write.c >>index 4f96699e363..472c206f64c 100644 >>--- a/fs/fat/fat_write.c >>+++ b/fs/fat/fat_write.c >>@@ -10,6 +10,7 @@ >> #include >> #include >> #include >>+#include >> #include >> #include >> #include >>@@ -17,16 +18,6 @@ >> #include >> #include "fat.c" >> >>-static void uppercase(char *str, int len) >>-{ >>- int i; >>- >>- for (i = 0; i < len; i++) { >>- *str = toupper(*str); >>- str++; >>- } >>-} We should not use toupper() here. We have to consider the FAT charset defined by CONFIG_FAT_DEFAULT_CODEPAGE. Best regards Heinrich >>- >> static int total_sector; >> static int disk_write(__u32 block, __u32 nr_blocks, void *buf) >> { >>@@ -65,7 +56,7 @@ static void set_name(dir_entry *dirent, const char >>*filename) >> return; >> >> strcpy(s_name, filename); >>- uppercase(s_name, len); >>+ str_to_upper(s_name, s_name, len); >> >> period = strchr(s_name, '.'); >> if (period == NULL) { >>diff --git a/include/vsprintf.h b/include/vsprintf.h >>index 56844dd2de8..d3f1292ae4d 100644 >>--- a/include/vsprintf.h >>+++ b/include/vsprintf.h >>@@ -222,4 +222,16 @@ bool str2long(const char *p, ulong *num); >> * @hz: Value to convert >> */ >> char *strmhz(char *buf, unsigned long hz); >>+ >>+/** >>+ * str_to_upper() - Convert a string to upper case >>+ * >>+ * This simply uses toupper() on each character of the string. >>+ * >>+ * @in: String to convert (must be large enough to hold the output >>string) >>+ * @out: Converting string >>+ * @len: Number of characters to convert (-1 for all) >>+ */ >>+void str_to_upper(const char *in, char *out, int len); > >Wouldn't we prefer size_t like in other string functions for len? > > >>+ >> #endif >>diff --git a/lib/strto.c b/lib/strto.c >>index 55ff9f7437d..563892a52c0 100644 >>--- a/lib/strto.c >>+++ b/lib/strto.c >>@@ -163,3 +163,11 @@ long trailing_strtol(const char *str) >> { >> return trailing_strtoln(str, NULL); >> } >>+ >>+void str_to_upper(const char *in, char *out, int len) >>+{ >>+ while ((len == -1 || len-- > 0) && *in) > >Why a special logic for -1? Just use SIZE_MAX when calling the >function. > >>+ *out++ = toupper(*in++); >>+ if (len && !*in) >>+ *out = '\0'; > >We shouldn't check len and *in twice in the loop. > >Please, add a test for > >str_to_upper("", SIZE_MAX) which your code is handling incorrectly. - >You miss copying 0x00 here. > >Best regards > >Heinrich > >>+} >>diff --git a/test/str_ut.c b/test/str_ut.c >>index 0ce6b055792..09726b076fe 100644 >>--- a/test/str_ut.c >>+++ b/test/str_ut.c >>@@ -19,36 +19,73 @@ static const char str3[] = "0xbI'm sorry you're >>alive."; >> /* Declare a new str test */ >> #define STR_TEST(_name, _flags) UNIT_TEST(_name, _flags, str_test) >> >>+static int str_test_upper(struct unit_test_state *uts) >>+{ >>+ char out[TEST_STR_SIZE]; >>+ >>+ /* Make sure it adds a terminator */ >>+ out[strlen(str1)] = 'a'; >>+ str_to_upper(str1, out, -1); >>+ ut_asserteq_str("I'M SORRY I'M LATE.", out); >>+ >>+ /* In-place operation */ >>+ strcpy(out, str2); >>+ str_to_upper(out, out, -1); >>+ ut_asserteq_str("1099ABNO, DON'T BOTHER APOLOGISING.", out); >>+ >>+ /* Limited length */ >>+ str_to_upper(str1, out, 7); >>+ ut_asserteq_str("I'M SORO, DON'T BOTHER APOLOGISING.", out); >>+ >>+ /* In-place with limited length */ >>+ strcpy(out, str2); >>+ str_to_upper(out, out, 7); >>+ ut_asserteq_str("1099ABNo, don't bother apologising.", out); >>+ >>+ return 0; >>+} >>+STR_TEST(str_test_upper, 0); >>+ >>static int run_strtoul(struct unit_test_state *uts, const char *str, >>int base, >>- ulong expect_val, int expect_endp_offset) >>+ ulong expect_val, int expect_endp_offset, bool upper) >> { >>+ char out[TEST_STR_SIZE]; >> char *endp; >> ulong val; >> >>- val = simple_strtoul(str, &endp, base); >>+ strcpy(out, str); >>+ if (upper) >>+ str_to_upper(out, out, -1); >>+ >>+ val = simple_strtoul(out, &endp, base); >> ut_asserteq(expect_val, val); >>- ut_asserteq(expect_endp_offset, endp - str); >>+ ut_asserteq(expect_endp_offset, endp - out); >> >> return 0; >> } >> >> static int str_simple_strtoul(struct unit_test_state *uts) >> { >>- /* Base 10 and base 16 */ >>- ut_assertok(run_strtoul(uts, str2, 10, 1099, 4)); >>- ut_assertok(run_strtoul(uts, str2, 16, 0x1099ab, 6)); >>- >>- /* Invalid string */ >>- ut_assertok(run_strtoul(uts, str1, 10, 0, 0)); >>- >>- /* Base 0 */ >>- ut_assertok(run_strtoul(uts, str1, 0, 0, 0)); >>- ut_assertok(run_strtoul(uts, str2, 0, 1099, 4)); >>- ut_assertok(run_strtoul(uts, str3, 0, 0xb, 3)); >>- >>- /* Base 2 */ >>- ut_assertok(run_strtoul(uts, str1, 2, 0, 0)); >>- ut_assertok(run_strtoul(uts, str2, 2, 2, 2)); >>+ int upper; >>+ >>+ /* Check that it is case-insentive */ >>+ for (upper = 0; upper < 2; upper++) { >>+ /* Base 10 and base 16 */ >>+ ut_assertok(run_strtoul(uts, str2, 10, 1099, 4, upper)); >>+ ut_assertok(run_strtoul(uts, str2, 16, 0x1099ab, 6, upper)); >>+ >>+ /* Invalid string */ >>+ ut_assertok(run_strtoul(uts, str1, 10, 0, 0, upper)); >>+ >>+ /* Base 0 */ >>+ ut_assertok(run_strtoul(uts, str1, 0, 0, 0, upper)); >>+ ut_assertok(run_strtoul(uts, str2, 0, 1099, 4, upper)); >>+ ut_assertok(run_strtoul(uts, str3, 0, 0xb, 3, upper)); >>+ >>+ /* Base 2 */ >>+ ut_assertok(run_strtoul(uts, str1, 2, 0, 0, upper)); >>+ ut_assertok(run_strtoul(uts, str2, 2, 2, 2, upper)); >>+ } >> >> /* Check endp being NULL */ >> ut_asserteq(1099, simple_strtoul(str2, NULL, 0));