[U-Boot] [PATCH 0/2] lib: errno: check for unsupported error number

errno_str() should not return a random pointer for unknown error codes.
Provide a unit test for errno_str().
Heinrich Schuchardt (2): lib: errno: check for unsupported error number test: provide test for errno_str()
lib/errno_str.c | 8 ++++++- test/lib/Makefile | 1 + test/lib/test_errno_str.c | 50 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 58 insertions(+), 1 deletion(-) create mode 100644 test/lib/test_errno_str.c
-- 2.23.0

If errno_str() is called with an unsupported error number, do not return a random pointer but a reasonable text.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/errno_str.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/errno_str.c b/lib/errno_str.c index fbe56395d1..2e5f4a887d 100644 --- a/lib/errno_str.c +++ b/lib/errno_str.c @@ -136,6 +136,8 @@ static const char * const errno_message[] = { ERRNO_MSG(EDQUOT, "Quota exceeded"), ERRNO_MSG(ENOMEDIUM, "No medium found"), ERRNO_MSG(EMEDIUMTYPE, "Wrong medium type"), + /* Message for unsupported error numbers */ + ERRNO_MSG(0, "Unknown error"), };
const char *errno_str(int errno) @@ -143,5 +145,9 @@ const char *errno_str(int errno) if (errno >= 0) return errno_message[0];
- return errno_message[abs(errno)]; + errno = -errno; + if (errno >= ARRAY_SIZE(errno_message)) + errno = ARRAY_SIZE(errno_message) - 1; + + return errno_message[errno]; } -- 2.23.0

On Sun, 6 Oct 2019 at 06:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
If errno_str() is called with an unsupported error number, do not return a random pointer but a reasonable text.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
lib/errno_str.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
Reviewed-by: Simon Glass sjg@chromium.org

If errno_str() is called with an unsupported error number, do not return a random pointer but a reasonable text.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- lib/errno_str.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/lib/errno_str.c b/lib/errno_str.c index fbe56395d1..2e5f4a887d 100644 --- a/lib/errno_str.c +++ b/lib/errno_str.c @@ -136,6 +136,8 @@ static const char * const errno_message[] = { ERRNO_MSG(EDQUOT, "Quota exceeded"), ERRNO_MSG(ENOMEDIUM, "No medium found"), ERRNO_MSG(EMEDIUMTYPE, "Wrong medium type"), + /* Message for unsupported error numbers */ + ERRNO_MSG(0, "Unknown error"), };
const char *errno_str(int errno) @@ -143,5 +145,9 @@ const char *errno_str(int errno) if (errno >= 0) return errno_message[0];
- return errno_message[abs(errno)]; + errno = -errno; + if (errno >= ARRAY_SIZE(errno_message)) + errno = ARRAY_SIZE(errno_message) - 1; + + return errno_message[errno]; } -- 2.23.0

On 10/6/19 2:33 PM, Heinrich Schuchardt wrote:
If errno_str() is called with an unsupported error number, do not return a random pointer but a reasonable text.
Signed-off-by: Heinrich Schuchardtxypron.glpk@gmx.de
This mail was sent by mistake. The correct patch is
[PATCH 1/2] lib: errno: check for unsupported error number https://lists.denx.de/pipermail/u-boot/2019-October/385618.html

Provide a unit test for errno_str(). Test that known and unknown error numbers are handled correctly.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de --- test/lib/Makefile | 1 + test/lib/test_errno_str.c | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/lib/test_errno_str.c
diff --git a/test/lib/Makefile b/test/lib/Makefile index 308c61708e..b13aaca7ce 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -6,3 +6,4 @@ obj-y += cmd_ut_lib.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o +obj-$(CONFIG_ERRNO_STR) += test_errno_str.o diff --git a/test/lib/test_errno_str.c b/test/lib/test_errno_str.c new file mode 100644 index 0000000000..77072d04f9 --- /dev/null +++ b/test/lib/test_errno_str.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de + * + * Unit tests for memory functions + * + * The architecture dependent implementations run through different lines of + * code depending on the alignment and length of memory regions copied or set. + * This has to be considered in testing. + */ + +#include <common.h> +#include <command.h> +#include <errno.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h> + +static const char errno_str_ok[] = "Success"; +static const char errno_str_enomem[] = "Out of memory"; +static const char errno_str_unknown[] = "Unknown error"; + +/** + * lib_errno_str() - unit test for errno_str() + * + * Test errno_str() with varied alignment and length of the copied buffer. + * + * @uts: unit test state + * Return: 0 = success, 1 = failure + */ +static int lib_errno_str(struct unit_test_state *uts) +{ + const char *msg; + + msg = errno_str(1); + ut_asserteq_str(errno_str_ok, msg); + + msg = errno_str(0); + ut_asserteq_str(errno_str_ok, msg); + + msg = errno_str(-ENOMEM); + ut_asserteq_str(errno_str_enomem, msg); + + msg = errno_str(-99999); + ut_asserteq_str(errno_str_unknown, msg); + + return 0; +} + +LIB_TEST(lib_errno_str, 0); -- 2.23.0

Hi Heinrich,
On Sun, 6 Oct 2019 at 06:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide a unit test for errno_str(). Test that known and unknown error numbers are handled correctly.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
test/lib/Makefile | 1 + test/lib/test_errno_str.c | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/lib/test_errno_str.c
diff --git a/test/lib/Makefile b/test/lib/Makefile index 308c61708e..b13aaca7ce 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -6,3 +6,4 @@ obj-y += cmd_ut_lib.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o +obj-$(CONFIG_ERRNO_STR) += test_errno_str.o diff --git a/test/lib/test_errno_str.c b/test/lib/test_errno_str.c new file mode 100644 index 0000000000..77072d04f9 --- /dev/null +++ b/test/lib/test_errno_str.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de
- Unit tests for memory functions
- The architecture dependent implementations run through different lines of
- code depending on the alignment and length of memory regions copied or set.
- This has to be considered in testing.
- */
+#include <common.h> +#include <command.h> +#include <errno.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h>
+static const char errno_str_ok[] = "Success"; +static const char errno_str_enomem[] = "Out of memory"; +static const char errno_str_unknown[] = "Unknown error";
I think in a test it is better to open-code the strings below rather than put them in separate constants.
Regards, Simon

On 10/13/19 5:03 PM, Simon Glass wrote:
Hi Heinrich,
On Sun, 6 Oct 2019 at 06:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide a unit test for errno_str(). Test that known and unknown error numbers are handled correctly.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
test/lib/Makefile | 1 + test/lib/test_errno_str.c | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/lib/test_errno_str.c
diff --git a/test/lib/Makefile b/test/lib/Makefile index 308c61708e..b13aaca7ce 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -6,3 +6,4 @@ obj-y += cmd_ut_lib.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o +obj-$(CONFIG_ERRNO_STR) += test_errno_str.o diff --git a/test/lib/test_errno_str.c b/test/lib/test_errno_str.c new file mode 100644 index 0000000000..77072d04f9 --- /dev/null +++ b/test/lib/test_errno_str.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de
- Unit tests for memory functions
- The architecture dependent implementations run through different lines of
- code depending on the alignment and length of memory regions copied or set.
- This has to be considered in testing.
- */
+#include <common.h> +#include <command.h> +#include <errno.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h>
+static const char errno_str_ok[] = "Success"; +static const char errno_str_enomem[] = "Out of memory"; +static const char errno_str_unknown[] = "Unknown error";
I think in a test it is better to open-code the strings below rather than put them in separate constants.
Thanks for reviewing.
The string errno_str_ok is used twice. That is why I opted for constants. test/compression.c is also using predefined constants. So it is not without precedent.
But this is only a matter of taste.
@Tom Which way do you want it (as maintainer for 'THE REST')?
Best regards
Heinrich

On Sun, Oct 13, 2019 at 05:36:06PM +0200, Heinrich Schuchardt wrote:
On 10/13/19 5:03 PM, Simon Glass wrote:
Hi Heinrich,
On Sun, 6 Oct 2019 at 06:33, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
Provide a unit test for errno_str(). Test that known and unknown error numbers are handled correctly.
Signed-off-by: Heinrich Schuchardt xypron.glpk@gmx.de
test/lib/Makefile | 1 + test/lib/test_errno_str.c | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+) create mode 100644 test/lib/test_errno_str.c
diff --git a/test/lib/Makefile b/test/lib/Makefile index 308c61708e..b13aaca7ce 100644 --- a/test/lib/Makefile +++ b/test/lib/Makefile @@ -6,3 +6,4 @@ obj-y += cmd_ut_lib.o obj-y += hexdump.o obj-y += lmb.o obj-y += string.o +obj-$(CONFIG_ERRNO_STR) += test_errno_str.o diff --git a/test/lib/test_errno_str.c b/test/lib/test_errno_str.c new file mode 100644 index 0000000000..77072d04f9 --- /dev/null +++ b/test/lib/test_errno_str.c @@ -0,0 +1,50 @@ +// SPDX-License-Identifier: GPL-2.0+ +/*
- Copyright (c) 2019 Heinrich Schuchardt xypron.glpk@gmx.de
- Unit tests for memory functions
- The architecture dependent implementations run through different lines of
- code depending on the alignment and length of memory regions copied or set.
- This has to be considered in testing.
- */
+#include <common.h> +#include <command.h> +#include <errno.h> +#include <test/lib.h> +#include <test/test.h> +#include <test/ut.h>
+static const char errno_str_ok[] = "Success"; +static const char errno_str_enomem[] = "Out of memory"; +static const char errno_str_unknown[] = "Unknown error";
I think in a test it is better to open-code the strings below rather than put them in separate constants.
Thanks for reviewing.
The string errno_str_ok is used twice. That is why I opted for constants. test/compression.c is also using predefined constants. So it is not without precedent.
But this is only a matter of taste.
@Tom Which way do you want it (as maintainer for 'THE REST')?
I think the compression test is special and otherwise yes, we should open-code them and let the compiler do its thing.
participants (3)
-
Heinrich Schuchardt
-
Simon Glass
-
Tom Rini