public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Will Dyson <will_dyson@pobox.com>
To: Alexander Viro <viro@math.psu.edu>
Cc: linux-kernel <linux-kernel@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org
Subject: Re: [CFT] BeFS filesystem 0.6
Date: Tue, 25 Dec 2001 00:54:51 -0500	[thread overview]
Message-ID: <3C2814AB.9060700@pobox.com> (raw)
In-Reply-To: <Pine.GSO.4.21.0112241625120.28049-100000@weyl.math.psu.edu>

Alexander Viro wrote:

 
> Umm...  Obvious comments:
> 
> a) typedef struct super_block vfs_sb;  Please don't.
> 
> b) in inode.c:
> 	inode->i_mode = (umode_t) raw_inode->mode;
> is wrong.  It's guaranteed bug either on big- or little-endian boxen.
> Same for mtime, uid and gid.  befs_count_blocks() also needs cleanup.
> 
> c) befs_read_block()...  Erm.  Why bother with extra layer of abstraction?
> 
> d) befs_readdir().  You _are_ guaranteed that inode is non-NULL, you put
> pointer to befs_dir_operations only is S_ISDIR is true and S_ISDIR is
> mutually exclusive with S_ISLNK.
> 
> e) are there sparse files on BeFS?  If yes - you want to make BH_Mapped
> conditional in get_block() (set it if block is present, don't if it's a
> hole in file).
> 
> f) befs_arch().  You probably want to make that an option...
> 
> g) endianness problems in read_super()...
> 
> h) TODO: use line breaks ;-)



Hi Al. Thanks for taking the time to respond.


A & B) I originaly had plans to make the driver work under a variety of 
different VFSs (maybe freebsd and atheos) and to have user-space tools 
(like the old mtools for DOS) for people on other OSs. That was the 
motivation behind those extra abstractions, but I've actually kinda lost 
interest in the idea. I'll just take 'em out if it really offends 
people's asthetic sense. Perhaps kernel drivers just weren't meant to be 
portable.

B) The PPC and x86 versions of BeOS each write their filesystem metadata 
in native byte-order. So this would only be a problem with disks that 
were formatted on a big-endian system and read on a little-endian (or 
the other way around, of course). I've been meaning to handle that case 
"someday".

D) Ok. I killed that test.

E) To my knowledge, there are not sparse files on BeFS.

F) Ideally, I would detect both the endianness of the filesystem and the 
host, and refuse to mount if they differ (or byte-swap the metadata). 
Can I rely on #ifdef __LITTLE_ENDIAN to detect the endianness I am 
running on?

G) Same thing as B, I think.

H) Yeah. I despise editors that won't line-wrap for you. But this is 
unix, so I guess I gotta accomodate them. Is makeing sure they don't 
overflow 80 columns enough? Or should that be 78?

 > befs_count_blocks() also needs cleanup.

How so, please?

-- 
Will Dyson


  reply	other threads:[~2001-12-25  5:55 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-12-24 20:58 [CFT] BeFS filesystem 0.6 Will Dyson
2001-12-24 21:59 ` Alexander Viro
2001-12-25  5:54   ` Will Dyson [this message]
2001-12-25  6:07     ` Alexander Viro

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3C2814AB.9060700@pobox.com \
    --to=will_dyson@pobox.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=viro@math.psu.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox