On Thu, 18 Aug 2022 11:51:19 -0000, Klemens Nanni wrote:
> I've checked all usage combination of flags and arguments, the new code
> does what the new synopsis says.
I agree with the general direction but have one concern. See inline.
- todd
> Index: installboot.c
> ===================================================================
> RCS file: /cvs/src/usr.sbin/installboot/installboot.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 installboot.c
> --- installboot.c 20 Jul 2021 14:51:56 -0000 1.14
> +++ installboot.c 18 Aug 2022 11:42:43 -0000
> @@ -31,17 +31,16 @@ int prepare;
> int stages;
> int verbose;
>
> -char *root = "/";
> +char *root;
If you don't initalize root here, a NULL pointer will be used later
when the -v option is not also used. You could also kill the useless
strdup of optarg when the -r option is used.
> char *stage1;
> char *stage2;
>
> static __dead void
> usage(void)
> {
> - extern char *__progname;
> -
> - fprintf(stderr, "usage: %s [-npv] [-r root] disk [stage1%s]\n",
> - __progname, (stages >= 2) ? " [stage2]" : "");
> + fprintf(stderr, "usage:\t%1$s [-nv] [-r root] disk [stage1%2$s]\n"
> + "\t%1$s [-nv] -p disk\n",
> + getprogname(), (stages >= 2) ? " [stage2]" : "");
>
> exit(1);
> }
> @@ -80,6 +79,8 @@ main(int argc, char **argv)
>
> if (argc < 1 || argc > stages + 1)
> usage();
> + if (prepare && (root != NULL || argc > 1))
> + usage();
>
> dev = argv[0];
> if (argc > 1)
> @@ -87,9 +88,21 @@ main(int argc, char **argv)
> if (argc > 2)
> stage2 = argv[2];
>
> + if ((devfd = opendev(dev, (nowrite ? O_RDONLY : O_RDWR), OPENDEV_PART,
> + &realdev)) == -1)
> + err(1, "open: %s", realdev);
> +
> + if (prepare) {
> + md_prepareboot(devfd, realdev);
> + return 0;
> + }
> +
> /* Prefix stages with root, unless they were user supplied. */
> - if (verbose)
> + if (verbose) {
> + if (root == NULL)
> + root = "/";
> fprintf(stderr, "Using %s as root\n", root);
> + }
> if (argc <= 1 && stage1 != NULL) {
> stage1 = fileprefix(root, stage1);
With your diff root may be NULL when calling fileprefix().
> if (stage1 == NULL)
> @@ -101,10 +114,6 @@ main(int argc, char **argv)
> exit(1);
> }
>
> - if ((devfd = opendev(dev, (nowrite ? O_RDONLY : O_RDWR), OPENDEV_PART,
> - &realdev)) == -1)
> - err(1, "open: %s", realdev);
> -
> if (verbose) {
> fprintf(stderr, "%s bootstrap on %s\n",
> (nowrite ? "would install" : "installing"), realdev);
> @@ -118,11 +127,6 @@ main(int argc, char **argv)
> }
>
> md_loadboot();
> -
> - if (prepare) {
> - md_prepareboot(devfd, realdev);
> - return 0;
> - }
>
> #ifdef SOFTRAID
> sr_installboot(devfd, dev);
>
