On 10/29/18 1:11 PM, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
>
> Add a self test for crc32 into xfs_io so that xfstests can check the
> operation of the (potentially cross-compiled) package binaries by
> isolating the self test code to a header file that can be included by
> the build system self test and xfs_io.
>
> Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> ---
> include/crc32selftest.h | 706 +++++++++++++++++++++++++++++++++++++++++++++++
> io/Makefile | 8 -
> io/crc32selftest.c | 51 +++
> io/init.c | 1
> io/io.h | 1
> libfrog/crc32.c | 669 ---------------------------------------------
needs a manpage change as well I think.
> 6 files changed, 764 insertions(+), 672 deletions(-)
> create mode 100644 include/crc32selftest.h
> create mode 100644 io/crc32selftest.c
>
...
> diff --git a/io/crc32selftest.c b/io/crc32selftest.c
> new file mode 100644
> index 00000000..c0b4b2f4
> --- /dev/null
> +++ b/io/crc32selftest.c
> @@ -0,0 +1,51 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright (c) 2018 Oracle, Inc.
> + * All Rights Reserved.
> + */
> +
> +#include "platform_defs.h"
> +#include "command.h"
> +#include "init.h"
> +#include "io.h"
> +#include "crc32c.h"
> +#include "crc32selftest.h"
> +
> +static int
> +crc32selftest_f(
> + int argc,
> + char **argv)
> +{
> + int c;
> + int errors;
> +
> + while ((c = getopt(argc, argv, "")) != EOF) {
> + switch (c) {
> + default:
> + fprintf(stderr,
> +_("Bad option for crc32selftest command.\n"));
> + return 0;
> + }
> + }
I think we can drop the getopt bits altogether because:
> + errors = crc32c_test();
> + return errors == 0 ? 0 : 1;
> +}
> +
> +static const cmdinfo_t crc32selftest_cmd = {
> + .name = "crc32selftest",
> + .cfunc = crc32selftest_f,
> + .argmin = 0,
> + .argmax = 0,
This command structure doesn't even let us get there:
# io/xfs_io -c "crc32selftest foo"
bad argument count 1 to crc32selftest, expected 0 arguments
# io/xfs_io -c "crc32selftest -f"
bad argument count 1 to crc32selftest, expected 0 arguments
and other "argmax" functions (freeze, thaw, close, munmap...) don't bother
with getopt at all.
> + .canpush = 0,
> + .args = "",
drop this I think, otherwise we get a weird space in usage:
crc32selftest -- crc32c self test
vs
crc32selftest -- crc32c self test
because
if (ct->args)
printf("%s ", ct->args);
printf("-- %s\n", ct->oneline);
> + .flags = CMD_FLAG_ONESHOT | CMD_FLAG_FOREIGN_OK |
> + CMD_NOFILE_OK | CMD_NOMAP_OK,
> + .oneline = N_("crc32c self test"),
> +};
> +
> +void
> +crc32selftest_init(void)
> +{
> + add_command(&crc32selftest_cmd);
> +}
ok is it better to call this crc32c selftest or crc32 selftest? Seems a little inconsistent in the naming.
> diff --git a/io/init.c b/io/init.c
> index b5eade39..53b754e4 100644
> --- a/io/init.c
> +++ b/io/init.c
> @@ -85,6 +85,7 @@ init_commands(void)
> sync_range_init();
> truncate_init();
> utimes_init();
> + crc32selftest_init();
> }
>
> /*
> diff --git a/io/io.h b/io/io.h
> index 9278ad00..847b4759 100644
> --- a/io/io.h
> +++ b/io/io.h
> @@ -182,3 +182,4 @@ extern void log_writes_init(void);
>
> extern void scrub_init(void);
> extern void repair_init(void);
> +extern void crc32selftest_init(void);
