Fw: RE: [patch] combine mount_udf(8) with kiconv(3)

[R. Imura]

Hi,

Yes, as Dave pointed,

+	iov[i++].iov_len = strlen(iov[1].iov_base) + 1;

should be

+	iov[i].iov_len = strlen(iov[i].iov_base) + 1;
+	i++;

Thanks.

- R. Imura


----- Forwarded message from Dave Hart <davehart at davehart.com> -----

Date: Tue, 4 Nov 2003 20:03:15 -0000
From: "Dave Hart" <davehart at davehart.com>
Subject: RE: [patch] combine mount_udf(8) with kiconv(3)
To: "R. Imura" <imura at ryu16.org>

I don't know the right protocol or etiquette for patch feedback so I'm
sending this to you without cc'ing any lists or individuals, but please
share it freely.

Here's a snippet of the diff:

Index: sbin/mount_udf/mount_udf.c
===================================================================
RCS file: /home/ncvs/src/sbin/mount_udf/mount_udf.c,v
retrieving revision 1.6
diff -u -r1.6 mount_udf.c
--- sbin/mount_udf/mount_udf.c	21 Aug 2002 18:11:17 -0000	1.6
+++ sbin/mount_udf/mount_udf.c	2 Nov 2003 00:29:39 -0000
@@ -46,10 +46,15 @@
 
 #include <sys/cdio.h>
 #include <sys/file.h>
+#include <sys/iconv.h>
 #include <sys/param.h>
+#include <sys/linker.h>
+#include <sys/module.h>
 #include <sys/mount.h>
 #include <sys/uio.h>
 
+#include <fs/udf/udf_mount.h>
+
 #include <err.h>
 #include <errno.h>
 #include <stdlib.h>
@@ -66,18 +71,21 @@
 	{ NULL, 0, 0, 0 }
 };
 
+int	set_charset(char **, char **, const char *);
 void	usage(void);
 
 int
 main(int argc, char **argv)
 {
-	struct iovec iov[6];
-	int ch, mntflags, opts;
+	struct iovec iov[12];
+	int ch, i, mntflags, opts, udf_flags;
 	char *dev, *dir, mntpath[MAXPATHLEN];
+	char *cs_disk, *cs_local;
 	int verbose;
 
-	mntflags = opts = verbose = 0;
-	while ((ch = getopt(argc, argv, "o:v")) != -1)
+	i = mntflags = opts = udf_flags = verbose = 0;
+	cs_disk = cs_local = NULL;
+	while ((ch = getopt(argc, argv, "o:vC:")) != -1)
 		switch (ch) {
 		case 'o':
 			getmntopts(optarg, mopts, &mntflags, &opts);
@@ -85,6 +93,11 @@
 		case 'v':
 			verbose++;
 			break;
+		case 'C':
+			if (set_charset(&cs_disk, &cs_local, optarg) ==
-1)
+				err(EX_OSERR, "udf_iconv");
+			udf_flags |= UDFMNT_KICONV;
+			break;
 		case '?':
 		default:
 			usage();
@@ -110,27 +123,70 @@
 	 */
 	mntflags |= MNT_RDONLY;
 
-	iov[0].iov_base = "fstype";
-	iov[0].iov_len = sizeof("fstype");
-	iov[1].iov_base = "udf";
-	iov[1].iov_len = strlen(iov[1].iov_base) + 1;
-	iov[2].iov_base = "fspath";
-	iov[2].iov_len = sizeof("fspath");
-	iov[3].iov_base = mntpath;
-	iov[3].iov_len = strlen(mntpath) + 1;
-	iov[4].iov_base = "from";
-	iov[4].iov_len = sizeof("from");
-	iov[5].iov_base = dev;
-	iov[5].iov_len = strlen(dev) + 1;
-	if (nmount(iov, 6, mntflags) < 0)
+	iov[i].iov_base = "fstype";
+	iov[i++].iov_len = sizeof("fstype");
+	iov[i].iov_base = "udf";
+	iov[i++].iov_len = strlen(iov[1].iov_base) + 1;
+	iov[i].iov_base = "fspath";
+	iov[i++].iov_len = sizeof("fspath");
+	iov[i].iov_base = mntpath;
+	iov[i++].iov_len = strlen(mntpath) + 1;
+	iov[i].iov_base = "from";
+	iov[i++].iov_len = sizeof("from");
+	iov[i].iov_base = dev;
+	iov[i++].iov_len = strlen(dev) + 1;
+	iov[i].iov_base = "flags";
+	iov[i++].iov_len = sizeof("flags");
+	iov[i].iov_base = &udf_flags;
+	iov[i++].iov_len = sizeof(udf_flags);
+	if (udf_flags & UDFMNT_KICONV) {
+		iov[i].iov_base = "cs_disk";
+		iov[i++].iov_len = sizeof("cs_disk") + 1;
+		iov[i].iov_base = cs_disk;
+		iov[i++].iov_len = strlen(cs_disk) + 1;
+		iov[i].iov_base = "cs_local";
+		iov[i++].iov_len = sizeof("cs_local") + 1;
+		iov[i].iov_base = cs_local;
+		iov[i++].iov_len = strlen(cs_local) + 1;
+	}
+	if (nmount(iov, i, mntflags) < 0)
 		err(1, "%s", dev);
 	exit(0);
 }

I think 

+	iov[i++].iov_len = strlen(iov[1].iov_base) + 1;

should be

+	iov[i++].iov_len = strlen(iov[i].iov_base) + 1;

or perhaps

+	iov[i].iov_len = strlen(iov[i].iov_base) + 1;
+     i++;

or equivalent.  Currently i == 1 for that code, so it should be
functionally identical.


Cheers,

Dave Hart

----- End forwarded message -----