Date: Sat, 1 Sep 2007 21:53:08 +0800 From: "Tz-Huan Huang" <tzhuan@csie.org> To: "Bruce Evans" <brde@optusnet.com.au> Cc: bugs@freebsd.org, fs@freebsd.org Subject: Re: msdosfs not MPSAFE Message-ID: <6a7033710709010653i2e43f517y953885b67160c1cd@mail.gmail.com> In-Reply-To: <20070831092907.K8647@besplex.bde.org> References: <20070712084115.GA2200@deviant.kiev.zoral.com.ua> <20070721233613.Q3366@besplex.bde.org> <20070804075730.GP2738@deviant.kiev.zoral.com.ua> <20070808004001.O926@besplex.bde.org> <20070807170259.GJ2738@deviant.kiev.zoral.com.ua> <20070810133946.H769@besplex.bde.org> <20070810124153.GW2738@deviant.kiev.zoral.com.ua> <20070814154812.J24186@besplex.bde.org> <6a7033710708300835n5ac95063o6331ede5fcc80662@mail.gmail.com> <20070831092907.K8647@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
2007/8/31, Bruce Evans <brde@optusnet.com.au>:
> This is too late. I just got approval to commit my version mailed
> here on 10 Aug, and will commit it today. That version also removes
> the globals, but doesn't change the algorithm so much. Your version
> seems to be simpler but has too many style bugs for me. If you want
> to do more work on this, please adjust your patch to apply to the
> version that I will commit and clean it up.
(sorry that I forgot to replay to all in the previous mail)
Ok, the attachment is the new version.
I think it's worth committing because it doesn't only solve the global
variables problem but also solve the kiconv problem. In original
interface the win2unixchr() return a 2-byte int thus it's impossible to
convert from UTF-16BE to UTF-8 because the size of a character in
UTF-8 may be from 1 byte to 6 bytes.
Tz-Huan Huang
[-- Attachment #2 --]
diff -urN msdosfs.orig/direntry.h msdosfs/direntry.h
--- msdosfs.orig/direntry.h 2007-09-01 06:29:55.000000000 +0800
+++ msdosfs/direntry.h 2007-09-01 21:17:25.000000000 +0800
@@ -133,27 +133,18 @@
#define DD_YEAR_SHIFT 9
#ifdef _KERNEL
-struct mbnambuf {
- size_t nb_len;
- int nb_last_id;
- char nb_buf[WIN_MAXLEN + 1];
-};
-
struct dirent;
struct msdosfsmount;
-char *mbnambuf_flush(struct mbnambuf *nbp, struct dirent *dp);
-void mbnambuf_init(struct mbnambuf *nbp);
-void mbnambuf_write(struct mbnambuf *nbp, char *name, int id);
int dos2unixfn(u_char dn[11], u_char *un, int lower,
struct msdosfsmount *pmp);
int unix2dosfn(const u_char *un, u_char dn[12], size_t unlen, u_int gen,
struct msdosfsmount *pmp);
-int unix2winfn(const u_char *un, size_t unlen, struct winentry *wep, int cnt,
- int chksum, struct msdosfsmount *pmp);
-int winChkName(struct mbnambuf *nbp, const u_char *un, size_t unlen,
+int unix2winfn(const u_char *un, size_t unlen, struct winentry *wep,
+ int cnt, int chksum, struct msdosfsmount *pmp);
+int winChkName(const u_char *un, size_t unlen, struct dirent *dirbufptr,
int chksum, struct msdosfsmount *pmp);
-int win2unixfn(struct mbnambuf *nbp, struct winentry *wep, int chksum,
+int win2unixfn(struct winentry *wep, struct dirent *dirbufptr, int chksum,
struct msdosfsmount *pmp);
u_int8_t winChksum(struct direntry *dep);
int winSlotCnt(const u_char *un, size_t unlen, struct msdosfsmount *pmp);
diff -urN msdosfs.orig/msdosfs_conv.c msdosfs/msdosfs_conv.c
--- msdosfs.orig/msdosfs_conv.c 2007-09-01 06:29:55.000000000 +0800
+++ msdosfs/msdosfs_conv.c 2007-09-01 21:34:11.000000000 +0800
@@ -52,6 +52,7 @@
#include <sys/systm.h>
#include <sys/dirent.h>
#include <sys/iconv.h>
+#include <sys/malloc.h>
#include <sys/mount.h>
#include <fs/msdosfs/bpb.h>
@@ -61,10 +62,14 @@
extern struct iconv_functions *msdosfs_iconv;
static int mbsadjpos(const char **, size_t, size_t, int, int, void *handle);
-static u_int16_t dos2unixchr(const u_char **, size_t *, int, struct msdosfsmount *);
+static u_int16_t dos2unixchr(const u_char **, size_t *, int,
+ struct msdosfsmount *);
static u_int16_t unix2doschr(const u_char **, size_t *, struct msdosfsmount *);
-static u_int16_t win2unixchr(u_int16_t, struct msdosfsmount *);
-static u_int16_t unix2winchr(const u_char **, size_t *, int, struct msdosfsmount *);
+static size_t win2unixchr(u_int16_t wc, struct dirent *dirbufptr,
+ struct msdosfsmount *pmp);
+static u_int16_t unix2winchr(const u_char **, size_t *, int,
+ struct msdosfsmount *);
+static void strreverse(char *, size_t);
/*
* 0 - character disallowed in long file name.
@@ -596,38 +601,33 @@
* Returns the checksum or -1 if no match
*/
int
-winChkName(nbp, un, unlen, chksum, pmp)
- struct mbnambuf *nbp;
+winChkName(un, unlen, dirbufptr, chksum, pmp)
const u_char *un;
size_t unlen;
+ struct dirent *dirbufptr;
int chksum;
struct msdosfsmount *pmp;
{
size_t len;
u_int16_t c1, c2;
u_char *np;
- struct dirent dirbuf;
- /*
- * We already have winentry in *nbp.
- */
- if (!mbnambuf_flush(nbp, &dirbuf) || dirbuf.d_namlen == 0)
- return -1;
+ if (dirbufptr->d_namlen == 0)
+ return (-1);
#ifdef MSDOSFS_DEBUG
- printf("winChkName(): un=%s:%d,d_name=%s:%d\n", un, unlen,
- dirbuf.d_name,
- dirbuf.d_namlen);
+ printf("winChkName(): un=%s:%d,d_name=%s:%d\n", un, unlen,
+ dirbufptr->d_name, dirbufptr->d_namlen);
#endif
/*
* Compare the name parts
*/
- len = dirbuf.d_namlen;
+ len = dirbufptr->d_namlen;
if (unlen != len)
- return -2;
+ return (-2);
- for (np = dirbuf.d_name; unlen > 0 && len > 0;) {
+ for (np = dirbufptr->d_name; unlen > 0 && len > 0;) {
/*
* Comparison must be case insensitive, because FAT disallows
* to look up or create files in case sensitive even when
@@ -636,24 +636,24 @@
c1 = unix2winchr((const u_char **)&np, &len, LCASE_BASE, pmp);
c2 = unix2winchr(&un, &unlen, LCASE_BASE, pmp);
if (c1 != c2)
- return -2;
+ return (-2);
}
- return chksum;
+ return (chksum);
}
/*
- * Convert Win95 filename to dirbuf.
+ * Convert Win95 filename to dirbufptr->d_name.
* Returns the checksum or -1 if impossible
*/
int
-win2unixfn(nbp, wep, chksum, pmp)
- struct mbnambuf *nbp;
+win2unixfn(wep, dirbufptr, chksum, pmp)
struct winentry *wep;
+ struct dirent *dirbufptr;
int chksum;
struct msdosfsmount *pmp;
{
+ const size_t d_namlen = dirbufptr->d_namlen; /* save for reverse */
u_int8_t *cp;
- u_int8_t *np, name[WIN_CHARS * 2 + 1];
u_int16_t code;
int i;
@@ -674,22 +674,19 @@
/*
* Convert the name parts
*/
- np = name;
for (cp = wep->wePart1, i = sizeof(wep->wePart1)/2; --i >= 0;) {
code = (cp[1] << 8) | cp[0];
switch (code) {
case 0:
- *np = '\0';
- mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
- return chksum;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (chksum);
case '/':
- *np = '\0';
- return -1;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (-1);
default:
- code = win2unixchr(code, pmp);
- if (code & 0xff00)
- *np++ = code >> 8;
- *np++ = code;
+ code = win2unixchr(code, dirbufptr, pmp);
+ if (code != 0)
+ return (-1);
break;
}
cp += 2;
@@ -698,17 +695,15 @@
code = (cp[1] << 8) | cp[0];
switch (code) {
case 0:
- *np = '\0';
- mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
- return chksum;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (chksum);
case '/':
- *np = '\0';
- return -1;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (-1);
default:
- code = win2unixchr(code, pmp);
- if (code & 0xff00)
- *np++ = code >> 8;
- *np++ = code;
+ code = win2unixchr(code, dirbufptr, pmp);
+ if (code != 0)
+ return (-1);
break;
}
cp += 2;
@@ -717,24 +712,28 @@
code = (cp[1] << 8) | cp[0];
switch (code) {
case 0:
- *np = '\0';
- mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
- return chksum;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (chksum);
case '/':
- *np = '\0';
- return -1;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+ return (-1);
default:
- code = win2unixchr(code, pmp);
- if (code & 0xff00)
- *np++ = code >> 8;
- *np++ = code;
+ code = win2unixchr(code, dirbufptr, pmp);
+ if (code != 0)
+ return (-1);
break;
}
cp += 2;
}
- *np = '\0';
- mbnambuf_write(nbp, name, (wep->weCnt & WIN_CNT) - 1);
- return chksum;
+ dirbufptr->d_name[dirbufptr->d_namlen] = '\0';
+
+ /* swap the dirbufptr->d_name */
+ strreverse(dirbufptr->d_name, dirbufptr->d_namlen);
+ strreverse(dirbufptr->d_name, dirbufptr->d_namlen - d_namlen);
+ strreverse(dirbufptr->d_name + dirbufptr->d_namlen - d_namlen,
+ d_namlen);
+
+ return (chksum);
}
/*
@@ -949,10 +948,10 @@
/*
* Convert Windows char to Local char
*/
-static u_int16_t
-win2unixchr(u_int16_t wc, struct msdosfsmount *pmp)
+static size_t
+win2unixchr(u_int16_t wc, struct dirent *dirbufptr, struct msdosfsmount *pmp)
{
- u_char *inp, *outp, inbuf[3], outbuf[3];
+ u_char *inp, *outp, inbuf[3];
size_t ilen, olen, len;
if (wc == 0)
@@ -963,31 +962,44 @@
inbuf[1] = (u_char)wc;
inbuf[2] = '\0';
- ilen = olen = len = 2;
+ ilen = 2;
+ olen = len = sizeof(dirbufptr->d_name) - dirbufptr->d_namlen - 1;
inp = inbuf;
- outp = outbuf;
+ outp = dirbufptr->d_name + dirbufptr->d_namlen;
+
msdosfs_iconv->convchr(pmp->pm_w2u, (const char **)&inp, &ilen,
- (char **)&outp, &olen);
- len -= olen;
+ (char **)&outp, &olen);
- /*
- * return '?' if failed to convert
- */
- if (len == 0) {
- wc = '?';
- return (wc);
- }
+ dirbufptr->d_namlen += len - olen;
- wc = 0;
- while(len--)
- wc |= (*(outp - len - 1) & 0xff) << (len << 3);
- return (wc);
+ if (ilen != 0) {
+ if (olen == 0)
+ goto too_long; /* longer than MAXNAMLEN */
+ else
+ goto failed; /* convert failed */
+ }
+
+ goto ok;
}
- if (wc & 0xff00)
- wc = '?';
+ if (sizeof(dirbufptr->d_name) <= dirbufptr->d_namlen + 2)
+ goto too_long; /* longer then MAXNAMLEN */
+ else if (wc & 0xff00)
+ goto failed; /* illegal character? */
- return (wc);
+ dirbufptr->d_name[dirbufptr->d_namlen++] = (u_char)(wc>>8);
+ dirbufptr->d_name[dirbufptr->d_namlen++] = (u_char)wc;
+ goto ok;
+
+ok:
+ return (0);
+
+too_long:
+ return (1);
+
+failed:
+ dirbufptr->d_name[dirbufptr->d_namlen++] = '?';
+ return (2);
}
/*
@@ -1034,73 +1046,17 @@
}
/*
- * Initialize the temporary concatenation buffer.
- */
-void
-mbnambuf_init(struct mbnambuf *nbp)
-{
-
- nbp->nb_len = 0;
- nbp->nb_last_id = -1;
- nbp->nb_buf[sizeof(nbp->nb_buf) - 1] = '\0';
-}
-
-/*
- * Fill out our concatenation buffer with the given substring, at the offset
- * specified by its id. Since this function must be called with ids in
- * descending order, we take advantage of the fact that ASCII substrings are
- * exactly WIN_CHARS in length. For non-ASCII substrings, we shift all
- * previous (i.e. higher id) substrings upwards to make room for this one.
- * This only penalizes portions of substrings that contain more than
- * WIN_CHARS bytes when they are first encountered.
+ * Reverse c-string in length n, for example, "12345" -> "54321".
*/
-void
-mbnambuf_write(struct mbnambuf *nbp, char *name, int id)
+static void
+strreverse(char *str, size_t n)
{
- char *slot;
- size_t count, newlen;
-
- KASSERT(nbp->nb_len == 0 || id == nbp->nb_last_id - 1,
- ("non-decreasing id: id %d, last id %d", id, nbp->nb_last_id));
-
- /* Will store this substring in a WIN_CHARS-aligned slot. */
- slot = &nbp->nb_buf[id * WIN_CHARS];
- count = strlen(name);
- newlen = nbp->nb_len + count;
- if (newlen > WIN_MAXLEN || newlen > MAXNAMLEN) {
- printf("msdosfs: file name length %zu too large\n", newlen);
- return;
- }
-
- /* Shift suffix upwards by the amount length exceeds WIN_CHARS. */
- if (count > WIN_CHARS && nbp->nb_len != 0)
- bcopy(slot + WIN_CHARS, slot + count, nbp->nb_len);
-
- /* Copy in the substring to its slot and update length so far. */
- bcopy(name, slot, count);
- nbp->nb_len = newlen;
- nbp->nb_last_id = id;
-}
-
-/*
- * Take the completed string and use it to setup the struct dirent.
- * Be sure to always nul-terminate the d_name and then copy the string
- * from our buffer. Note that this function assumes the full string has
- * been reassembled in the buffer. If it's called before all substrings
- * have been written via mbnambuf_write(), the result will be incorrect.
- */
-char *
-mbnambuf_flush(struct mbnambuf *nbp, struct dirent *dp)
-{
-
- if (nbp->nb_len > sizeof(dp->d_name) - 1) {
- mbnambuf_init(nbp);
- return (NULL);
- }
- bcopy(&nbp->nb_buf[0], dp->d_name, nbp->nb_len);
- dp->d_name[nbp->nb_len] = '\0';
- dp->d_namlen = nbp->nb_len;
+ size_t i;
+ char c;
- mbnambuf_init(nbp);
- return (dp->d_name);
+ for (i = 0; i < n/2; ++i) {
+ c = str[i];
+ str[i] = str[n - i - 1];
+ str[n - i - 1] = c;
+ }
}
diff -urN msdosfs.orig/msdosfs_lookup.c msdosfs/msdosfs_lookup.c
--- msdosfs.orig/msdosfs_lookup.c 2007-09-01 06:29:55.000000000 +0800
+++ msdosfs/msdosfs_lookup.c 2007-09-01 21:35:00.000000000 +0800
@@ -54,6 +54,7 @@
#include <sys/mount.h>
#include <sys/namei.h>
#include <sys/vnode.h>
+#include <sys/dirent.h>
#include <fs/msdosfs/bpb.h>
#include <fs/msdosfs/direntry.h>
@@ -84,7 +85,6 @@
struct componentname *a_cnp;
} */ *ap;
{
- struct mbnambuf nb;
struct vnode *vdp = ap->a_dvp;
struct vnode **vpp = ap->a_vpp;
struct componentname *cnp = ap->a_cnp;
@@ -104,6 +104,7 @@
struct denode *tdp;
struct msdosfsmount *pmp;
struct buf *bp = 0;
+ struct dirent namebuf;
struct direntry *dep = NULL;
u_char dosfilename[12];
int flags = cnp->cn_flags;
@@ -186,7 +187,7 @@
* by cnp->cn_nameptr.
*/
tdp = NULL;
- mbnambuf_init(&nb);
+ namebuf.d_namlen = 0;
/*
* The outer loop ranges over the clusters that make up the
* directory. Note that the root directory is different from all
@@ -226,7 +227,7 @@
* Drop memory of previous long matches
*/
chksum = -1;
- mbnambuf_init(&nb);
+ namebuf.d_namlen = 0;
if (slotcount < wincnt) {
slotcount++;
@@ -251,16 +252,17 @@
if (pmp->pm_flags & MSDOSFSMNT_SHORTNAME)
continue;
- chksum = win2unixfn(&nb,
- (struct winentry *)dep, chksum,
- pmp);
+ chksum = win2unixfn(
+ (struct winentry *)dep, &namebuf,
+ chksum, pmp);
continue;
}
- chksum = winChkName(&nb,
+ chksum = winChkName(
(const u_char *)cnp->cn_nameptr, unlen,
- chksum, pmp);
+ &namebuf, chksum, pmp);
if (chksum == -2) {
+ namebuf.d_namlen = 0;
chksum = -1;
continue;
}
diff -urN msdosfs.orig/msdosfs_vnops.c msdosfs/msdosfs_vnops.c
--- msdosfs.orig/msdosfs_vnops.c 2007-09-01 06:29:55.000000000 +0800
+++ msdosfs/msdosfs_vnops.c 2007-09-01 21:30:40.000000000 +0800
@@ -1510,7 +1510,6 @@
u_long **a_cookies;
} */ *ap;
{
- struct mbnambuf nb;
int error = 0;
int diff;
long n;
@@ -1549,6 +1548,7 @@
/*
* To be safe, initialize dirbuf
*/
+ dirbuf.d_namlen = 0;
bzero(dirbuf.d_name, sizeof(dirbuf.d_name));
/*
@@ -1630,7 +1630,7 @@
}
}
- mbnambuf_init(&nb);
+ dirbuf.d_namlen = 0;
off = offset;
while (uio->uio_resid > 0) {
lbn = de_cluster(pmp, offset - bias);
@@ -1677,7 +1677,7 @@
*/
if (dentp->deName[0] == SLOT_DELETED) {
chksum = -1;
- mbnambuf_init(&nb);
+ dirbuf.d_namlen = 0;
continue;
}
@@ -1687,8 +1687,8 @@
if (dentp->deAttributes == ATTR_WIN95) {
if (pmp->pm_flags & MSDOSFSMNT_SHORTNAME)
continue;
- chksum = win2unixfn(&nb,
- (struct winentry *)dentp, chksum, pmp);
+ chksum = win2unixfn((struct winentry *)dentp,
+ &dirbuf, chksum, pmp);
continue;
}
@@ -1697,7 +1697,7 @@
*/
if (dentp->deAttributes & ATTR_VOLUME) {
chksum = -1;
- mbnambuf_init(&nb);
+ dirbuf.d_namlen = 0;
continue;
}
/*
@@ -1739,9 +1739,8 @@
((pmp->pm_flags & MSDOSFSMNT_SHORTNAME) ?
(LCASE_BASE | LCASE_EXT) : 0),
pmp);
- mbnambuf_init(&nb);
- } else
- mbnambuf_flush(&nb, &dirbuf);
+ }
+
chksum = -1;
dirbuf.d_reclen = GENERIC_DIRSIZ(&dirbuf);
if (uio->uio_resid < dirbuf.d_reclen) {
@@ -1761,6 +1760,7 @@
}
}
off = offset + sizeof(struct direntry);
+ dirbuf.d_namlen = 0;
}
brelse(bp);
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?6a7033710709010653i2e43f517y953885b67160c1cd>