Re: svn commit: r351319 - in head/usr.sbin/makefs: ffs msdos

[Ed Maste]

On Fri, 30 Aug 2019 at 00:29, Bruce Evans  wrote:
>
> On Wed, 21 Aug 2019, Ed Maste wrote:
>
> > Author: emaste
> > Date: Wed Aug 21 01:45:29 2019
> > New Revision: 351319
> > URL: https://svnweb.freebsd.org/changeset/base/351319
> >
> > Log:
> >  makefs: use `char *` not `void *` for buf b_data, drop casts in msdos
> >
> >  (The kernel uses caddr_t.)
>
> This is much better than churning the copy of the working code.

My eventual goal is to converge on a shared copy of these files,
generally moving to what is already done in the kernel's
implementation. If there's reasonable clean up or improvements to be
made in the kernel code I'm interested in doing so, but I'm not really
sure what to do with the caddr_ts there yet. I see that NetBSD removed
all uses of caddr_t.
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

Re: svn commit: r351319 - in head/usr.sbin/makefs: ffs msdos

[Bruce Evans]

On Wed, 21 Aug 2019, Ed Maste wrote:


Author: emaste
Date: Wed Aug 21 01:45:29 2019
New Revision: 351319
URL: https://svnweb.freebsd.org/changeset/base/351319

Log:
 makefs: use `char *` not `void *` for buf b_data, drop casts in msdos

 (The kernel uses caddr_t.)


This is much better than churning the copy of the working code.

However, the kernel type is still bogus.  caddr_t should not exist, and
should have been more like vm_offset_t to begin with.  void * is no good
as an opaque type for it, since void * is only good for C addresses within
a single address space, but caddr_t is supposed to be able to represent
anything in a (possibly multiple) "core" address space.  [u]intptr_t has
the same problem.  Bus space addresses are also withing a single address
space and in general need a tag for uniqueness.

caddr_t has been char * for too long, so too much code including buf b_data
depends on it being precisely char * or u_char * to work.  char * is an
honestly broken variant of void *.  It is similarly limited to a single
address space.  This is hard to fix.  It works for [Free]BSD since no arches
with more than a single address space are supported (i386 with PAE or
pae_mode=1 or pae_mode has a larger physical address space where addresses
aren't unique, but only the kernel can see this).

Bruce
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r351319 - in head/usr.sbin/makefs: ffs msdos

[Ed Maste]

Author: emaste
Date: Wed Aug 21 01:45:29 2019
New Revision: 351319
URL: https://svnweb.freebsd.org/changeset/base/351319

Log:
  makefs: use `char *` not `void *` for buf b_data, drop casts in msdos
  
  (The kernel uses caddr_t.)
  
  Suggested by: cem
  Reviewed by:  cem
  MFC with: r351273
  Sponsored by: The FreeBSD Foundation
  Differential Revision:https://reviews.freebsd.org/D21348

Modified:
  head/usr.sbin/makefs/ffs/buf.h
  head/usr.sbin/makefs/msdos/msdosfs_fat.c

Modified: head/usr.sbin/makefs/ffs/buf.h
==
--- head/usr.sbin/makefs/ffs/buf.h  Tue Aug 20 21:59:48 2019
(r351318)
+++ head/usr.sbin/makefs/ffs/buf.h  Wed Aug 21 01:45:29 2019
(r351319)
@@ -54,7 +54,7 @@ struct vnode {
 };
 
 struct buf {
-   void *  b_data;
+   char *  b_data;
longb_bufsize;
longb_bcount;
daddr_t b_blkno;

Modified: head/usr.sbin/makefs/msdos/msdosfs_fat.c
==
--- head/usr.sbin/makefs/msdos/msdosfs_fat.cTue Aug 20 21:59:48 2019
(r351318)
+++ head/usr.sbin/makefs/msdos/msdosfs_fat.cWed Aug 21 01:45:29 2019
(r351319)
@@ -220,9 +220,9 @@ pcbmap(struct denode *dep, u_long findcn, daddr_t *bnp
return (EIO);
}
if (FAT32(pmp))
-   cn = getulong((char *)bp->b_data + bo);
+   cn = getulong(bp->b_data + bo);
else
-   cn = getushort((char *)bp->b_data + bo);
+   cn = getushort(bp->b_data + bo);
if (FAT12(pmp) && (prevcn & 1))
cn >>= 4;
cn &= pmp->pm_fatmask;
@@ -502,9 +502,9 @@ fatentry(int function, struct msdosfsmount *pmp, u_lon
 
if (function & FAT_GET) {
if (FAT32(pmp))
-   readcn = getulong((char *)bp->b_data + bo);
+   readcn = getulong(bp->b_data + bo);
else
-   readcn = getushort((char *)bp->b_data + bo);
+   readcn = getushort(bp->b_data + bo);
if (FAT12(pmp) & (cn & 1))
readcn >>= 4;
readcn &= pmp->pm_fatmask;
@@ -516,7 +516,7 @@ fatentry(int function, struct msdosfsmount *pmp, u_lon
if (function & FAT_SET) {
switch (pmp->pm_fatmask) {
case FAT12_MASK:
-   readcn = getushort((char *)bp->b_data + bo);
+   readcn = getushort(bp->b_data + bo);
if (cn & 1) {
readcn &= 0x000f;
readcn |= newcontents << 4;
@@ -524,20 +524,20 @@ fatentry(int function, struct msdosfsmount *pmp, u_lon
readcn &= 0xf000;
readcn |= newcontents & 0xfff;
}
-   putushort((char *)bp->b_data + bo, readcn);
+   putushort(bp->b_data + bo, readcn);
break;
case FAT16_MASK:
-   putushort((char *)bp->b_data + bo, newcontents);
+   putushort(bp->b_data + bo, newcontents);
break;
case FAT32_MASK:
/*
 * According to spec we have to retain the
 * high order bits of the FAT entry.
 */
-   readcn = getulong((char *)bp->b_data + bo);
+   readcn = getulong(bp->b_data + bo);
readcn &= ~FAT32_MASK;
readcn |= newcontents & FAT32_MASK;
-   putulong((char *)bp->b_data + bo, readcn);
+   putulong(bp->b_data + bo, readcn);
break;
}
updatefats(pmp, bp, bn);
@@ -587,7 +587,7 @@ fatchain(struct msdosfsmount *pmp, u_long start, u_lon
newc = --count > 0 ? start : fillwith;
switch (pmp->pm_fatmask) {
case FAT12_MASK:
-   readcn = getushort((char *)bp->b_data + bo);
+   readcn = getushort(bp->b_data + bo);
if (start & 1) {
readcn &= 0xf000;
readcn |= newc & 0xfff;
@@ -595,20 +595,20 @@ fatchain(struct msdosfsmount *pmp, u_long start, u_lon
readcn &= 0x000f;
readcn |= newc << 4;
}
-   putushort((char *)bp->b_data + bo, readcn);
+