[v3] lib/test_printf.c: accept "ptrval" as valid result for plain 'p' tests

Message ID 20180613171840.29827-1-thierry.escande@linaro.org
State Accepted
Commit ce041c43f22298485122bab15c14d062383fbc67
Headers show
Series
  • [v3] lib/test_printf.c: accept "ptrval" as valid result for plain 'p' tests
Related show

Commit Message

Thierry Escande June 13, 2018, 5:18 p.m.
If the test_printf module is loaded before the crng is initialized, the
plain 'p' tests will fail because the printed address will not be hashed
and the buffer will contain "(____ptrval____)" or "(ptrval)" instead
(64-bit vs 32-bit).
Since we cannot wait for the crng to be initialized for an undefined
time, both plain 'p' tests now accept the strings "(____ptrval____)" or
"(ptrval)" as a valid result and print a warning message.

Signed-off-by: Thierry Escande <thierry.escande@linaro.org>

---

Changes in v3:
- Use "(____ptrval____)" for 64-bit
- Update commit message headline

Changes in v2:
- Remove wait_for_random_bytes() usage
- Remove Acked-by from Tobin as the proposed solution is not the same
  anymore.

 lib/test_printf.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

-- 
2.14.1

Comments

Andy Shevchenko June 14, 2018, 8:59 a.m. | #1
On Wed, Jun 13, 2018 at 8:18 PM, Thierry Escande
<thierry.escande@linaro.org> wrote:
> If the test_printf module is loaded before the crng is initialized, the

> plain 'p' tests will fail because the printed address will not be hashed

> and the buffer will contain "(____ptrval____)" or "(ptrval)" instead

> (64-bit vs 32-bit).

> Since we cannot wait for the crng to be initialized for an undefined

> time, both plain 'p' tests now accept the strings "(____ptrval____)" or

> "(ptrval)" as a valid result and print a warning message.

>


LGTM.
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>


> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>

> ---

>

> Changes in v3:

> - Use "(____ptrval____)" for 64-bit

> - Update commit message headline

>

> Changes in v2:

> - Remove wait_for_random_bytes() usage

> - Remove Acked-by from Tobin as the proposed solution is not the same

>   anymore.

>

>  lib/test_printf.c | 24 ++++++++++++++++++++++--

>  1 file changed, 22 insertions(+), 2 deletions(-)

>

> diff --git a/lib/test_printf.c b/lib/test_printf.c

> index 71ebfa43ad05..31f00b8ca657 100644

> --- a/lib/test_printf.c

> +++ b/lib/test_printf.c

> @@ -206,6 +206,7 @@ test_string(void)

>  #define PTR_WIDTH 16

>  #define PTR ((void *)0xffff0123456789ab)

>  #define PTR_STR "ffff0123456789ab"

> +#define PTR_VAL_NO_CRNG "(____ptrval____)"

>  #define ZEROS "00000000"       /* hex 32 zero bits */

>

>  static int __init

> @@ -216,7 +217,16 @@ plain_format(void)

>

>         nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);

>

> -       if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0)

> +       if (nchars != PTR_WIDTH)

> +               return -1;

> +

> +       if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {

> +               pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",

> +                       PTR_VAL_NO_CRNG);

> +               return 0;

> +       }

> +

> +       if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)

>                 return -1;

>

>         return 0;

> @@ -227,6 +237,7 @@ plain_format(void)

>  #define PTR_WIDTH 8

>  #define PTR ((void *)0x456789ab)

>  #define PTR_STR "456789ab"

> +#define PTR_VAL_NO_CRNG "(ptrval)"

>

>  static int __init

>  plain_format(void)

> @@ -245,7 +256,16 @@ plain_hash(void)

>

>         nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);

>

> -       if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0)

> +       if (nchars != PTR_WIDTH)

> +               return -1;

> +

> +       if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {

> +               pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",

> +                       PTR_VAL_NO_CRNG);

> +               return 0;

> +       }

> +

> +       if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0)

>                 return -1;

>

>         return 0;

> --

> 2.14.1

>




-- 
With Best Regards,
Andy Shevchenko
Petr Mladek June 15, 2018, 1:22 p.m. | #2
On Wed 2018-06-13 19:18:40, Thierry Escande wrote:
> If the test_printf module is loaded before the crng is initialized, the

> plain 'p' tests will fail because the printed address will not be hashed

> and the buffer will contain "(____ptrval____)" or "(ptrval)" instead

> (64-bit vs 32-bit).

> Since we cannot wait for the crng to be initialized for an undefined

> time, both plain 'p' tests now accept the strings "(____ptrval____)" or

> "(ptrval)" as a valid result and print a warning message.

> 

> Signed-off-by: Thierry Escande <thierry.escande@linaro.org>


Revieved-by: Petr Mladek <pmladek@suse.com>

I have pushed this into printk.git, branch for-4.19

Best Regards,
Petr

PS: The split into plain_hash() and plain_format() is weird and even
confusing. They both just checks different halfs of the same output.

It would make sense to merge them. It would also remove the duplicated
code and warning. Anyone interested into sending a followup patch? ;-)

Patch

diff --git a/lib/test_printf.c b/lib/test_printf.c
index 71ebfa43ad05..31f00b8ca657 100644
--- a/lib/test_printf.c
+++ b/lib/test_printf.c
@@ -206,6 +206,7 @@  test_string(void)
 #define PTR_WIDTH 16
 #define PTR ((void *)0xffff0123456789ab)
 #define PTR_STR "ffff0123456789ab"
+#define PTR_VAL_NO_CRNG "(____ptrval____)"
 #define ZEROS "00000000"	/* hex 32 zero bits */
 
 static int __init
@@ -216,7 +217,16 @@  plain_format(void)
 
 	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
 
-	if (nchars != PTR_WIDTH || strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
+	if (nchars != PTR_WIDTH)
+		return -1;
+
+	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
+			PTR_VAL_NO_CRNG);
+		return 0;
+	}
+
+	if (strncmp(buf, ZEROS, strlen(ZEROS)) != 0)
 		return -1;
 
 	return 0;
@@ -227,6 +237,7 @@  plain_format(void)
 #define PTR_WIDTH 8
 #define PTR ((void *)0x456789ab)
 #define PTR_STR "456789ab"
+#define PTR_VAL_NO_CRNG "(ptrval)"
 
 static int __init
 plain_format(void)
@@ -245,7 +256,16 @@  plain_hash(void)
 
 	nchars = snprintf(buf, PLAIN_BUF_SIZE, "%p", PTR);
 
-	if (nchars != PTR_WIDTH || strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
+	if (nchars != PTR_WIDTH)
+		return -1;
+
+	if (strncmp(buf, PTR_VAL_NO_CRNG, PTR_WIDTH) == 0) {
+		pr_warn("crng possibly not yet initialized. plain 'p' buffer contains \"%s\"",
+			PTR_VAL_NO_CRNG);
+		return 0;
+	}
+
+	if (strncmp(buf, PTR_STR, PTR_WIDTH) == 0)
 		return -1;
 
 	return 0;