On Tue, Jun 06, 2023 at 02:58:02PM +0530, Chandan Babu R wrote:
> We will need two variants of read_header(), show_info() and restore() helper
> functions to support two versions of metadump formats. To this end, A future
> commit will introduce a vector of function pointers to work with the two
> metadump formats. To have a common function signature for the function
> pointers, this commit replaces the first argument of the previously listed
> function pointers from "struct xfs_metablock *" with "void *".
>
> Signed-off-by: Chandan Babu R <chandan.babu@xxxxxxxxxx>
> ---
> mdrestore/xfs_mdrestore.c | 24 +++++++++++++++++-------
> 1 file changed, 17 insertions(+), 7 deletions(-)
>
> diff --git a/mdrestore/xfs_mdrestore.c b/mdrestore/xfs_mdrestore.c
> index 08f52527..5451a58b 100644
> --- a/mdrestore/xfs_mdrestore.c
> +++ b/mdrestore/xfs_mdrestore.c
> @@ -87,9 +87,11 @@ open_device(
>
> static void
> read_header(
> - struct xfs_metablock *mb,
> + void *header,
Should we be using a union here instead of a generic void pointer?
union xfs_mdrestore_headers {
__be32 magic;
struct xfs_metablock v1;
struct xfs_metadump_header v2;
};
Then you can do:
union xfs_mdrestore_headers headers;
fread(&headers.magic, sizeof(headers.magic), 1, md_fp));
switch (be32_to_cpu(headers.magic)) {
case XFS_MD_MAGIC_V1:
ret = dosomething_v1(&headers, ...);
break;
case XFS_MD_MAGIC_V2:
ret = dosomething_v2(&headers, ...);
break;
And there'll be at least *some* typechecking going on here.
> FILE *md_fp)
> {
> + struct xfs_metablock *mb = header;
> +
> mb->mb_magic = cpu_to_be32(XFS_MD_MAGIC_V1);
And no need for the casting:
static void
read_header_v1(
union xfs_mdrestore_headers *h,
FILE *md_fp)
{
fread(&h->v1.mb_count, sizeof(h->v1.mb_count), 1, md_fp);
...
}
static void
read_header_v2(
union xfs_mdrestore_headers *h,
FILE *md_fp)
{
fread(&h->v2.xmh_version,
sizeof(struct xfs_metadump_header) - offsetof(struct xfs_metadump_header, xmh_version),
1, md_fp);
...
}
--D
>
> if (fread((uint8_t *)mb + sizeof(mb->mb_magic),
> @@ -99,9 +101,11 @@ read_header(
>
> static void
> show_info(
> - struct xfs_metablock *mb,
> + void *header,
> const char *md_file)
> {
> + struct xfs_metablock *mb = header;
> +
> if (mb->mb_info & XFS_METADUMP_INFO_FLAGS) {
> printf("%s: %sobfuscated, %s log, %s metadata blocks\n",
> md_file,
> @@ -125,12 +129,13 @@ show_info(
> */
> static void
> restore(
> + void *header,
> FILE *md_fp,
> int ddev_fd,
> - int is_target_file,
> - const struct xfs_metablock *mbp)
> + int is_target_file)
> {
> struct xfs_metablock *metablock; /* header + index + blocks */
> + struct xfs_metablock *mbp;
> __be64 *block_index;
> char *block_buffer;
> int block_size;
> @@ -140,6 +145,8 @@ restore(
> xfs_sb_t sb;
> int64_t bytes_read;
>
> + mbp = header;
> +
> block_size = 1 << mbp->mb_blocklog;
> max_indices = (block_size - sizeof(xfs_metablock_t)) / sizeof(__be64);
>
> @@ -269,6 +276,7 @@ main(
> int c;
> bool is_target_file;
> uint32_t magic;
> + void *header;
> struct xfs_metablock mb;
>
> mdrestore.show_progress = false;
> @@ -321,15 +329,17 @@ main(
>
> switch (be32_to_cpu(magic)) {
> case XFS_MD_MAGIC_V1:
> - read_header(&mb, src_f);
> + header = &mb;
> break;
> default:
> fatal("specified file is not a metadata dump\n");
> break;
> }
>
> + read_header(header, src_f);
> +
> if (mdrestore.show_info) {
> - show_info(&mb, argv[optind]);
> + show_info(header, argv[optind]);
>
> if (argc - optind == 1)
> exit(0);
> @@ -340,7 +350,7 @@ main(
> /* check and open target */
> dst_fd = open_device(argv[optind], &is_target_file);
>
> - restore(src_f, dst_fd, is_target_file, &mb);
> + restore(header, src_f, dst_fd, is_target_file);
>
> close(dst_fd);
> if (src_f != stdin)
> --
> 2.39.1
>
