Am April 8, 2020 4:18:12 AM UTC schrieb Heinrich Schuchardt
<xypron.g...@gmx.de>:
>Am April 8, 2020 3:00:38 AM UTC schrieb Simon Glass <s...@chromium.org>:
>>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 <s...@chromium.org>
>>---
>>
>>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 <config.h>
>> #include <fat.h>
>> #include <malloc.h>
>>+#include <vsprintf.h>
>> #include <asm/byteorder.h>
>> #include <part.h>
>> #include <linux/ctype.h>
>>@@ -17,16 +18,6 @@
>> #include <linux/math64.h>
>> #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));
