The following series of patches implement API call needed to handle
64-bit block numbers. Im concentrating mainly in providing the API
call first and if the interfaces are sane, we can go ahead and start
using them in the rest of libext2fs and the user space programs.
I've run checkpatch and make check on each individual patch in the
series so they should be ready to add to the main repository if the
interfaces are acceptable.
-JRS
From: Jose R. Santos <[email protected]>
Use blk64_t for blocks in struct ext2_file.
The ext2_file structure is never exposed through the libext2fs API so
it is safe to use 64-bit blocks for blockno and physclock without
breaking the ABI.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/fileio.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/lib/ext2fs/fileio.c b/lib/ext2fs/fileio.c
index 8bf99fb..4b9e1ce 100644
--- a/lib/ext2fs/fileio.c
+++ b/lib/ext2fs/fileio.c
@@ -25,8 +25,8 @@ struct ext2_file {
struct ext2_inode inode;
int flags;
__u64 pos;
- blk_t blockno;
- blk_t physblock;
+ blk64_t blockno;
+ blk64_t physblock;
char *buf;
};
@@ -116,9 +116,9 @@ errcode_t ext2fs_file_flush(ext2_file_t file)
* Allocate it.
*/
if (!file->physblock) {
- retval = ext2fs_bmap(fs, file->ino, &file->inode,
+ retval = ext2fs_bmap2(fs, file->ino, &file->inode,
BMAP_BUFFER, file->ino ? BMAP_ALLOC : 0,
- file->blockno, &file->physblock);
+ file->blockno, 0, &file->physblock);
if (retval)
return retval;
}
@@ -168,8 +168,8 @@ static errcode_t load_buffer(ext2_file_t file, int dontfill)
errcode_t retval;
if (!(file->flags & EXT2_FILE_BUF_VALID)) {
- retval = ext2fs_bmap(fs, file->ino, &file->inode,
- BMAP_BUFFER, 0, file->blockno,
+ retval = ext2fs_bmap2(fs, file->ino, &file->inode,
+ BMAP_BUFFER, 0, file->blockno, 0,
&file->physblock);
if (retval)
return retval;
From: Jose R. Santos <[email protected]>
Add ext2_off64_t type.
The ext2_off_t is u32. Creating a new 64-bit ext2_off64_t for 64bit
offsets.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/ext2fs.h | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 7a1d966..4eb14d4 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -68,6 +68,7 @@ typedef __u32 blk_t;
typedef __u64 blk64_t;
typedef __u32 dgrp_t;
typedef __u32 ext2_off_t;
+typedef __u64 ext2_off64_t;
typedef __s64 e2_blkcnt_t;
typedef __u32 ext2_dirhash_t;
From: Jose R. Santos <[email protected]>
Add 64-bit dirblock interface.
Add new ext2fs_(read|write)_dir_block3() routines that take blk64_t as
an input.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/dirblock.c | 23 +++++++++++++++++------
lib/ext2fs/ext2fs.h | 4 ++++
2 files changed, 21 insertions(+), 6 deletions(-)
diff --git a/lib/ext2fs/dirblock.c b/lib/ext2fs/dirblock.c
index fb20fa0..c45c6ce 100644
--- a/lib/ext2fs/dirblock.c
+++ b/lib/ext2fs/dirblock.c
@@ -19,7 +19,7 @@
#include "ext2_fs.h"
#include "ext2fs.h"
-errcode_t ext2fs_read_dir_block2(ext2_filsys fs, blk_t block,
+errcode_t ext2fs_read_dir_block3(ext2_filsys fs, blk64_t block,
void *buf, int flags EXT2FS_ATTR((unused)))
{
errcode_t retval;
@@ -28,7 +28,7 @@ errcode_t ext2fs_read_dir_block2(ext2_filsys fs, blk_t block,
unsigned int name_len, rec_len;
- retval = io_channel_read_blk(fs->io, block, 1, buf);
+ retval = io_channel_read_blk64(fs->io, block, 1, buf);
if (retval)
return retval;
@@ -58,14 +58,20 @@ errcode_t ext2fs_read_dir_block2(ext2_filsys fs, blk_t block,
return retval;
}
+errcode_t ext2fs_read_dir_block2(ext2_filsys fs, blk_t block,
+ void *buf, int flags EXT2FS_ATTR((unused)))
+{
+ return ext2fs_read_dir_block3(fs, block, buf, flags);
+}
+
errcode_t ext2fs_read_dir_block(ext2_filsys fs, blk_t block,
void *buf)
{
- return ext2fs_read_dir_block2(fs, block, buf, 0);
+ return ext2fs_read_dir_block3(fs, block, buf, 0);
}
-errcode_t ext2fs_write_dir_block2(ext2_filsys fs, blk_t block,
+errcode_t ext2fs_write_dir_block3(ext2_filsys fs, blk64_t block,
void *inbuf, int flags EXT2FS_ATTR((unused)))
{
#ifdef WORDS_BIGENDIAN
@@ -95,7 +101,7 @@ errcode_t ext2fs_write_dir_block2(ext2_filsys fs, blk_t block,
if (flags & EXT2_DIRBLOCK_V2_STRUCT)
dirent->name_len = ext2fs_swab16(dirent->name_len);
}
- retval = io_channel_write_blk(fs->io, block, 1, buf);
+ retval = io_channel_write_blk64(fs->io, block, 1, buf);
ext2fs_free_mem(&buf);
return retval;
#else
@@ -103,10 +109,15 @@ errcode_t ext2fs_write_dir_block2(ext2_filsys fs, blk_t block,
#endif
}
+errcode_t ext2fs_write_dir_block2(ext2_filsys fs, blk_t block,
+ void *inbuf, int flags EXT2FS_ATTR((unused)))
+{
+ return ext2fs_write_dir_block3(fs, block, inbuf, flags);
+}
errcode_t ext2fs_write_dir_block(ext2_filsys fs, blk_t block,
void *inbuf)
{
- return ext2fs_write_dir_block2(fs, block, inbuf, 0);
+ return ext2fs_write_dir_block3(fs, block, inbuf, 0);
}
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 4eb14d4..3f640cf 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -752,10 +752,14 @@ extern errcode_t ext2fs_read_dir_block(ext2_filsys fs, blk_t block,
void *buf);
extern errcode_t ext2fs_read_dir_block2(ext2_filsys fs, blk_t block,
void *buf, int flags);
+extern errcode_t ext2fs_read_dir_block3(ext2_filsys fs, blk64_t block,
+ void *buf, int flags);
extern errcode_t ext2fs_write_dir_block(ext2_filsys fs, blk_t block,
void *buf);
extern errcode_t ext2fs_write_dir_block2(ext2_filsys fs, blk_t block,
void *buf, int flags);
+extern errcode_t ext2fs_write_dir_block3(ext2_filsys fs, blk64_t block,
+ void *buf, int flags);
/* dirhash.c */
extern errcode_t ext2fs_dirhash(int version, const char *name, int len,
From: Jose R. Santos <[email protected]>
Add 64-bit alloc_stats interface.
Add new ext2fs_block_alloc_stats2() routine that takes blk64_t as an
input.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/alloc_stats.c | 10 +++++++++-
lib/ext2fs/ext2fs.h | 1 +
2 files changed, 10 insertions(+), 1 deletions(-)
diff --git a/lib/ext2fs/alloc_stats.c b/lib/ext2fs/alloc_stats.c
index 3956528..2183507 100644
--- a/lib/ext2fs/alloc_stats.c
+++ b/lib/ext2fs/alloc_stats.c
@@ -54,13 +54,16 @@ void ext2fs_inode_alloc_stats(ext2_filsys fs, ext2_ino_t ino, int inuse)
ext2fs_inode_alloc_stats2(fs, ino, inuse, 0);
}
-void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse)
+void ext2fs_block_alloc_stats2(ext2_filsys fs, blk64_t blk, int inuse)
{
+ /*FIXME: Not blk64_t clean */
int group = ext2fs_group_of_blk(fs, blk);
if (inuse > 0)
+ /*FIXME: Not blk64_t clean */
ext2fs_mark_block_bitmap(fs->block_map, blk);
else
+ /*FIXME: Not blk64_t clean */
ext2fs_unmark_block_bitmap(fs->block_map, blk);
fs->group_desc[group].bg_free_blocks_count -= inuse;
fs->group_desc[group].bg_flags &= ~EXT2_BG_BLOCK_UNINIT;
@@ -70,3 +73,8 @@ void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse)
ext2fs_mark_super_dirty(fs);
ext2fs_mark_bb_dirty(fs);
}
+
+void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse)
+{
+ ext2fs_block_alloc_stats2(fs, blk, inuse);
+}
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 3f640cf..8107d94 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -554,6 +554,7 @@ void ext2fs_inode_alloc_stats(ext2_filsys fs, ext2_ino_t ino, int inuse);
void ext2fs_inode_alloc_stats2(ext2_filsys fs, ext2_ino_t ino,
int inuse, int isdir);
void ext2fs_block_alloc_stats(ext2_filsys fs, blk_t blk, int inuse);
+void ext2fs_block_alloc_stats2(ext2_filsys fs, blk64_t blk, int inuse);
/* alloc_tables.c */
extern errcode_t ext2fs_allocate_tables(ext2_filsys fs);
From: Jose R. Santos <[email protected]>
Add 64-bit alloc interface.
Add new ext2fs_new_block2(), ext2fs_get_free_blocks2() and
ext2fs_alloc_block2() that take and return blk64_t.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/alloc.c | 59 +++++++++++++++++++++++++++++++++++++++++----------
lib/ext2fs/ext2fs.h | 8 +++++++
2 files changed, 55 insertions(+), 12 deletions(-)
diff --git a/lib/ext2fs/alloc.c b/lib/ext2fs/alloc.c
index 65f3ea1..3dda7f4 100644
--- a/lib/ext2fs/alloc.c
+++ b/lib/ext2fs/alloc.c
@@ -73,10 +73,10 @@ errcode_t ext2fs_new_inode(ext2_filsys fs, ext2_ino_t dir,
* Stupid algorithm --- we now just search forward starting from the
* goal. Should put in a smarter one someday....
*/
-errcode_t ext2fs_new_block(ext2_filsys fs, blk_t goal,
- ext2fs_block_bitmap map, blk_t *ret)
+errcode_t ext2fs_new_block2(ext2_filsys fs, blk64_t goal,
+ ext2fs_block_bitmap map, blk64_t *ret)
{
- blk_t i;
+ blk64_t i;
EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
@@ -88,6 +88,7 @@ errcode_t ext2fs_new_block(ext2_filsys fs, blk_t goal,
goal = fs->super->s_first_data_block;
i = goal;
do {
+ /* FIXME: Not blk64_t clean */
if (!ext2fs_fast_test_block_bitmap(map, i)) {
*ret = i;
return 0;
@@ -99,15 +100,26 @@ errcode_t ext2fs_new_block(ext2_filsys fs, blk_t goal,
return EXT2_ET_BLOCK_ALLOC_FAIL;
}
+errcode_t ext2fs_new_block(ext2_filsys fs, blk_t goal,
+ ext2fs_block_bitmap map, blk_t *ret)
+{
+ errcode_t retval;
+ blk64_t val;
+ retval = ext2fs_new_block2(fs, goal, map, &val);
+ if (!retval)
+ *ret = (blk_t) val;
+ return retval;
+}
+
/*
* This function zeros out the allocated block, and updates all of the
* appropriate filesystem records.
*/
-errcode_t ext2fs_alloc_block(ext2_filsys fs, blk_t goal,
- char *block_buf, blk_t *ret)
+errcode_t ext2fs_alloc_block2(ext2_filsys fs, blk64_t goal,
+ char *block_buf, blk64_t *ret)
{
errcode_t retval;
- blk_t block;
+ blk64_t block;
char *buf = 0;
if (!block_buf) {
@@ -119,20 +131,21 @@ errcode_t ext2fs_alloc_block(ext2_filsys fs, blk_t goal,
memset(block_buf, 0, fs->blocksize);
if (!fs->block_map) {
+ /* FIXME: Not blk64_t clean */
retval = ext2fs_read_block_bitmap(fs);
if (retval)
goto fail;
}
- retval = ext2fs_new_block(fs, goal, 0, &block);
+ retval = ext2fs_new_block2(fs, goal, 0, &block);
if (retval)
goto fail;
- retval = io_channel_write_blk(fs->io, block, 1, block_buf);
+ retval = io_channel_write_blk64(fs->io, block, 1, block_buf);
if (retval)
goto fail;
- ext2fs_block_alloc_stats(fs, block, +1);
+ ext2fs_block_alloc_stats2(fs, block, 1);
*ret = block;
fail:
@@ -141,10 +154,21 @@ fail:
return retval;
}
-errcode_t ext2fs_get_free_blocks(ext2_filsys fs, blk_t start, blk_t finish,
- int num, ext2fs_block_bitmap map, blk_t *ret)
+errcode_t ext2fs_alloc_block(ext2_filsys fs, blk_t goal,
+ char *block_buf, blk_t *ret)
+{
+ errcode_t retval;
+ blk64_t val;
+ retval = ext2fs_alloc_block2(fs, goal, block_buf, &val);
+ if (!retval)
+ *ret = (blk_t) val;
+ return retval;
+}
+
+errcode_t ext2fs_get_free_blocks2(ext2_filsys fs, blk64_t start, blk64_t finish,
+ int num, ext2fs_block_bitmap map, blk64_t *ret)
{
- blk_t b = start;
+ blk64_t b = start;
EXT2_CHECK_MAGIC(fs, EXT2_ET_MAGIC_EXT2FS_FILSYS);
@@ -161,6 +185,7 @@ errcode_t ext2fs_get_free_blocks(ext2_filsys fs, blk_t start, blk_t finish,
do {
if (b+num-1 > fs->super->s_blocks_count)
b = fs->super->s_first_data_block;
+ /* FIXME: Not blk64_t clean */
if (ext2fs_fast_test_block_bitmap_range(map, b, num)) {
*ret = b;
return 0;
@@ -170,3 +195,13 @@ errcode_t ext2fs_get_free_blocks(ext2_filsys fs, blk_t start, blk_t finish,
return EXT2_ET_BLOCK_ALLOC_FAIL;
}
+errcode_t ext2fs_get_free_blocks(ext2_filsys fs, blk_t start, blk_t finish,
+ int num, ext2fs_block_bitmap map, blk_t *ret)
+{
+ errcode_t retval;
+ blk64_t val;
+ retval = ext2fs_get_free_blocks2(fs, start, finish, num, map, &val);
+ if(!retval)
+ *ret = (blk_t) val;
+ return retval;
+}
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 8107d94..80ccfd9 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -537,12 +537,20 @@ extern errcode_t ext2fs_new_inode(ext2_filsys fs, ext2_ino_t dir, int mode,
ext2fs_inode_bitmap map, ext2_ino_t *ret);
extern errcode_t ext2fs_new_block(ext2_filsys fs, blk_t goal,
ext2fs_block_bitmap map, blk_t *ret);
+extern errcode_t ext2fs_new_block2(ext2_filsys fs, blk64_t goal,
+ ext2fs_block_bitmap map, blk64_t *ret);
extern errcode_t ext2fs_get_free_blocks(ext2_filsys fs, blk_t start,
blk_t finish, int num,
ext2fs_block_bitmap map,
blk_t *ret);
+extern errcode_t ext2fs_get_free_blocks2(ext2_filsys fs, blk64_t start,
+ blk64_t finish, int num,
+ ext2fs_block_bitmap map,
+ blk64_t *ret);
extern errcode_t ext2fs_alloc_block(ext2_filsys fs, blk_t goal,
char *block_buf, blk_t *ret);
+extern errcode_t ext2fs_alloc_block2(ext2_filsys fs, blk64_t goal,
+ char *block_buf, blk64_t *ret);
/* alloc_sb.c */
extern int ext2fs_reserve_super_and_bgd(ext2_filsys fs,
From: Jose R. Santos <[email protected]>
Add 64-bit ext_attr interface.
Add ext2fs_read_ext_attr2(), ext2fs_write_ext_attr2() and
ext2fs_adjust_ea_refcount2() that take blk64_t as an input.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/ext2fs.h | 7 +++++++
lib/ext2fs/ext_attr.c | 31 ++++++++++++++++++++++++-------
2 files changed, 31 insertions(+), 7 deletions(-)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index 80ccfd9..f578c19 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -811,11 +811,18 @@ extern errcode_t ext2fs_expand_dir(ext2_filsys fs, ext2_ino_t dir);
extern __u32 ext2fs_ext_attr_hash_entry(struct ext2_ext_attr_entry *entry,
void *data);
extern errcode_t ext2fs_read_ext_attr(ext2_filsys fs, blk_t block, void *buf);
+extern errcode_t ext2fs_read_ext_attr2(ext2_filsys fs, blk64_t block,
+ void *buf);
extern errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block,
void *buf);
+extern errcode_t ext2fs_write_ext_attr2(ext2_filsys fs, blk64_t block,
+ void *buf);
extern errcode_t ext2fs_adjust_ea_refcount(ext2_filsys fs, blk_t blk,
char *block_buf,
int adjust, __u32 *newcount);
+extern errcode_t ext2fs_adjust_ea_refcount2(ext2_filsys fs, blk64_t blk,
+ char *block_buf,
+ int adjust, __u32 *newcount);
/* extent.c */
extern errcode_t ext2fs_extent_header_verify(void *ptr, int size);
diff --git a/lib/ext2fs/ext_attr.c b/lib/ext2fs/ext_attr.c
index 3d208ec..395d4c7 100644
--- a/lib/ext2fs/ext_attr.c
+++ b/lib/ext2fs/ext_attr.c
@@ -60,11 +60,11 @@ __u32 ext2fs_ext_attr_hash_entry(struct ext2_ext_attr_entry *entry, void *data)
#undef NAME_HASH_SHIFT
#undef VALUE_HASH_SHIFT
-errcode_t ext2fs_read_ext_attr(ext2_filsys fs, blk_t block, void *buf)
+errcode_t ext2fs_read_ext_attr2(ext2_filsys fs, blk64_t block, void *buf)
{
errcode_t retval;
- retval = io_channel_read_blk(fs->io, block, 1, buf);
+ retval = io_channel_read_blk64(fs->io, block, 1, buf);
if (retval)
return retval;
#ifdef WORDS_BIGENDIAN
@@ -73,7 +73,12 @@ errcode_t ext2fs_read_ext_attr(ext2_filsys fs, blk_t block, void *buf)
return 0;
}
-errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf)
+errcode_t ext2fs_read_ext_attr(ext2_filsys fs, blk_t block, void *buf)
+{
+ return ext2fs_read_ext_attr2(fs, block, buf);
+}
+
+errcode_t ext2fs_write_ext_attr2(ext2_filsys fs, blk64_t block, void *inbuf)
{
errcode_t retval;
char *write_buf;
@@ -88,7 +93,7 @@ errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf)
#else
write_buf = (char *) inbuf;
#endif
- retval = io_channel_write_blk(fs->io, block, 1, write_buf);
+ retval = io_channel_write_blk64(fs->io, block, 1, write_buf);
if (buf)
ext2fs_free_mem(&buf);
if (!retval)
@@ -96,10 +101,15 @@ errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf)
return retval;
}
+errcode_t ext2fs_write_ext_attr(ext2_filsys fs, blk_t block, void *inbuf)
+{
+ return ext2fs_write_ext_attr2(fs, block, inbuf);
+}
+
/*
* This function adjusts the reference count of the EA block.
*/
-errcode_t ext2fs_adjust_ea_refcount(ext2_filsys fs, blk_t blk,
+errcode_t ext2fs_adjust_ea_refcount2(ext2_filsys fs, blk64_t blk,
char *block_buf, int adjust,
__u32 *newcount)
{
@@ -118,7 +128,7 @@ errcode_t ext2fs_adjust_ea_refcount(ext2_filsys fs, blk_t blk,
block_buf = buf;
}
- retval = ext2fs_read_ext_attr(fs, blk, block_buf);
+ retval = ext2fs_read_ext_attr2(fs, blk, block_buf);
if (retval)
goto errout;
@@ -127,7 +137,7 @@ errcode_t ext2fs_adjust_ea_refcount(ext2_filsys fs, blk_t blk,
if (newcount)
*newcount = header->h_refcount;
- retval = ext2fs_write_ext_attr(fs, blk, block_buf);
+ retval = ext2fs_write_ext_attr2(fs, blk, block_buf);
if (retval)
goto errout;
@@ -136,3 +146,10 @@ errout:
ext2fs_free_mem(&buf);
return retval;
}
+
+errcode_t ext2fs_adjust_ea_refcount(ext2_filsys fs, blk_t blk,
+ char *block_buf, int adjust,
+ __u32 *newcount)
+{
+ return ext2fs_adjust_ea_refcount(fs, blk, block_buf, adjust, newcount);
+}
From: Jose R. Santos <[email protected]>
Add new blk64_t handling inline functions
Introduce inline fuctions to handle blk64_t and low/high values in
super blocks and inodes.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/ext2fs.h | 91 +++++++++++++++++++++++++++++++++++++++++++++++----
1 files changed, 84 insertions(+), 7 deletions(-)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index f578c19..ed99901 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1135,10 +1135,21 @@ extern void ext2fs_mark_ib_dirty(ext2_filsys fs);
extern void ext2fs_mark_bb_dirty(ext2_filsys fs);
extern int ext2fs_test_ib_dirty(ext2_filsys fs);
extern int ext2fs_test_bb_dirty(ext2_filsys fs);
+extern int ext2fs_group_of_blk2(ext2_filsys fs, blk64_t);
extern int ext2fs_group_of_blk(ext2_filsys fs, blk_t blk);
extern int ext2fs_group_of_ino(ext2_filsys fs, ext2_ino_t ino);
+extern blk64_t ext2fs_blocks_count(ext2_filsys fs);
+extern void ext2_blocks_count_set(ext2_filsys fs, blk64_t blk);
+extern blk64_t ext2_r_blocks_count(ext2_filsys fs);
+extern void ext2_r_blocks_count_set(ext2_filsys fs, blk64_t blk);
+extern blk64_t ext2_free_blocks_count(ext2_filsys fs);
+extern void ext2_free_blocks_count_set(ext2_filsys fs, blk64_t blk);
+extern blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group);
extern blk_t ext2fs_group_first_block(ext2_filsys fs, dgrp_t group);
+extern blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group);
extern blk_t ext2fs_group_last_block(ext2_filsys fs, dgrp_t group);
+extern blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
+ struct ext2_inode *inode);
extern blk_t ext2fs_inode_data_blocks(ext2_filsys fs,
struct ext2_inode *inode);
extern unsigned int ext2fs_div_ceil(unsigned int a, unsigned int b);
@@ -1299,12 +1310,17 @@ _INLINE_ int ext2fs_test_bb_dirty(ext2_filsys fs)
/*
* Return the group # of a block
*/
-_INLINE_ int ext2fs_group_of_blk(ext2_filsys fs, blk_t blk)
+_INLINE_ int ext2fs_group_of_blk2(ext2_filsys fs, blk64_t blk)
{
return (blk - fs->super->s_first_data_block) /
fs->super->s_blocks_per_group;
}
+_INLINE_ int ext2fs_group_of_blk(ext2_filsys fs, blk_t blk)
+{
+ return ext2fs_group_of_blk2(fs, blk);
+}
+
/*
* Return the group # of an inode number
*/
@@ -1313,31 +1329,91 @@ _INLINE_ int ext2fs_group_of_ino(ext2_filsys fs, ext2_ino_t ino)
return (ino - 1) / fs->super->s_inodes_per_group;
}
+_INLINE_ blk64_t ext2fs_blocks_count(ext2_filsys fs)
+{
+ return fs->super->s_blocks_count |
+ (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT ?
+ (__u64) fs->super->s_blocks_count_hi << 32 : 0);
+}
+
+_INLINE_ void ext2_blocks_count_set(ext2_filsys fs, blk64_t blk)
+{
+ fs->super->s_blocks_count = blk;
+ if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
+ fs->super->s_blocks_count_hi = (__u64) blk >> 32;
+}
+
+_INLINE_ blk64_t ext2_r_blocks_count(ext2_filsys fs)
+{
+ return fs->super->s_r_blocks_count |
+ (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT ?
+ (__u64) fs->super->s_r_blocks_count_hi << 32 : 0);
+}
+
+_INLINE_ void ext2_r_blocks_count_set(ext2_filsys fs, blk64_t blk)
+{
+ fs->super->s_r_blocks_count = blk;
+ if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
+ fs->super->s_r_blocks_count_hi = (__u64) blk >> 32;
+}
+
+_INLINE_ blk64_t ext2_free_blocks_count(ext2_filsys fs)
+{
+ return fs->super->s_free_blocks_count |
+ (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT ?
+ (__u64) fs->super->s_free_blocks_hi << 32 : 0);
+}
+
+_INLINE_ void ext2_free_blocks_count_set(ext2_filsys fs, blk64_t blk)
+{
+ fs->super->s_free_blocks_count = blk;
+ if (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT)
+ fs->super->s_free_blocks_hi = (__u64) blk >> 32;
+}
+
/*
* Return the first block (inclusive) in a group
*/
-_INLINE_ blk_t ext2fs_group_first_block(ext2_filsys fs, dgrp_t group)
+_INLINE_ blk64_t ext2fs_group_first_block2(ext2_filsys fs, dgrp_t group)
{
return fs->super->s_first_data_block +
(group * fs->super->s_blocks_per_group);
}
+_INLINE_ blk_t ext2fs_group_first_block(ext2_filsys fs, dgrp_t group)
+{
+ return ext2fs_group_first_block2(fs, group);
+}
+
/*
* Return the last block (inclusive) in a group
*/
-_INLINE_ blk_t ext2fs_group_last_block(ext2_filsys fs, dgrp_t group)
+_INLINE_ blk64_t ext2fs_group_last_block2(ext2_filsys fs, dgrp_t group)
{
return (group == fs->group_desc_count - 1 ?
- fs->super->s_blocks_count - 1 :
- ext2fs_group_first_block(fs, group) +
+ ext2fs_blocks_count(fs) - 1 :
+ ext2fs_group_first_block2(fs, group) +
(fs->super->s_blocks_per_group - 1));
}
+_INLINE_ blk_t ext2fs_group_last_block(ext2_filsys fs, dgrp_t group)
+{
+ return ext2fs_group_last_block2(fs, group);
+}
+
+_INLINE_ blk64_t ext2fs_inode_data_blocks2(ext2_filsys fs,
+ struct ext2_inode *inode)
+{
+ return (inode->i_blocks |
+ (fs->super->s_feature_incompat & EXT4_FEATURE_INCOMPAT_64BIT ?
+ (__u64)inode->osd2.linux2.l_i_blocks_hi << 32 : 0)) -
+ (inode->i_file_acl ? fs->blocksize >> 9 : 0);
+}
+
_INLINE_ blk_t ext2fs_inode_data_blocks(ext2_filsys fs,
struct ext2_inode *inode)
{
- return inode->i_blocks -
- (inode->i_file_acl ? fs->blocksize >> 9 : 0);
+ return ext2fs_inode_data_blocks2(fs, inode);
}
/*
@@ -1349,6 +1425,7 @@ _INLINE_ unsigned int ext2fs_div_ceil(unsigned int a, unsigned int b)
return 0;
return ((a - 1) / b) + 1;
}
+
#undef _INLINE_
#endif
From: Jose R. Santos <[email protected]>
Add 64-bit closefs interface.
Add new ext2fs_super_and_bgd_loc2() that returns blk64_t pointers.
The function now returns the number of blocks used by super block and
group descriptors since with flex_bg, it can no longer be assumed that
bitmaps and inode tables still resided within the block group.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/closefs.c | 94 ++++++++++++++++++++++++++++++++++----------------
lib/ext2fs/ext2fs.h | 6 +++
2 files changed, 69 insertions(+), 31 deletions(-)
diff --git a/lib/ext2fs/closefs.c b/lib/ext2fs/closefs.c
index 206faa6..cfb5ddc 100644
--- a/lib/ext2fs/closefs.c
+++ b/lib/ext2fs/closefs.c
@@ -47,30 +47,23 @@ int ext2fs_bg_has_super(ext2_filsys fs, int group_block)
/*
* This function returns the location of the superblock, block group
- * descriptors for a given block group. It currently returns the
- * number of free blocks assuming that inode table and allocation
- * bitmaps will be in the group. This is not necessarily the case
- * when the flex_bg feature is enabled, so callers should take care!
- * It was only really intended for use by mke2fs, and even there it's
- * not that useful. In the future, when we redo this function for
- * 64-bit block numbers, we should probably return the number of
- * blocks used by the super block and group descriptors instead.
- *
- * See also the comment for ext2fs_reserve_super_and_bgd()
+ * descriptors for a given block group. It returns the number of
+ * blocks used by super block and group descriptors.
*/
-int ext2fs_super_and_bgd_loc(ext2_filsys fs,
+int ext2fs_super_and_bgd_loc2(ext2_filsys fs,
dgrp_t group,
- blk_t *ret_super_blk,
- blk_t *ret_old_desc_blk,
- blk_t *ret_new_desc_blk,
+ blk64_t *ret_super_blk,
+ blk64_t *ret_old_desc_blk,
+ blk64_t *ret_new_desc_blk,
int *ret_meta_bg)
{
- blk_t group_block, super_blk = 0, old_desc_blk = 0, new_desc_blk = 0;
+ blk64_t group_block, super_blk = 0, old_desc_blk = 0, new_desc_blk = 0;
unsigned int meta_bg, meta_bg_size;
- blk_t numblocks, old_desc_blocks;
+ blk_t numblocks = 0;
+ blk64_t old_desc_blocks;
int has_super;
- group_block = ext2fs_group_first_block(fs, group);
+ group_block = ext2fs_group_first_block2(fs, group);
if (fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG)
old_desc_blocks = fs->super->s_first_meta_bg;
@@ -78,20 +71,11 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs,
old_desc_blocks =
fs->desc_blocks + fs->super->s_reserved_gdt_blocks;
- if (group == fs->group_desc_count-1) {
- numblocks = (fs->super->s_blocks_count -
- fs->super->s_first_data_block) %
- fs->super->s_blocks_per_group;
- if (!numblocks)
- numblocks = fs->super->s_blocks_per_group;
- } else
- numblocks = fs->super->s_blocks_per_group;
-
has_super = ext2fs_bg_has_super(fs, group);
if (has_super) {
super_blk = group_block;
- numblocks--;
+ numblocks++;
}
meta_bg_size = EXT2_DESC_PER_BLOCK(fs->super);
meta_bg = group / meta_bg_size;
@@ -100,7 +84,7 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs,
(meta_bg < fs->super->s_first_meta_bg)) {
if (has_super) {
old_desc_blk = group_block + 1;
- numblocks -= old_desc_blocks;
+ numblocks += old_desc_blocks;
}
} else {
if (((group % meta_bg_size) == 0) ||
@@ -109,11 +93,9 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs,
if (has_super)
has_super = 1;
new_desc_blk = group_block + has_super;
- numblocks--;
+ numblocks++;
}
}
-
- numblocks -= 2 + fs->inode_blocks_per_group;
if (ret_super_blk)
*ret_super_blk = super_blk;
@@ -126,6 +108,56 @@ int ext2fs_super_and_bgd_loc(ext2_filsys fs,
return (numblocks);
}
+/*
+ * This function returns the location of the superblock, block group
+ * descriptors for a given block group. It currently returns the
+ * number of free blocks assuming that inode table and allocation
+ * bitmaps will be in the group. This is not necessarily the case
+ * when the flex_bg feature is enabled, so callers should take care!
+ * It was only really intended for use by mke2fs, and even there it's
+ * not that useful.
+ *
+ * The ext2fs_super_and_bgd_loc2() function is 64-bit block number
+ * capable and returns the number of blocks used by super block and
+ * group descriptors.
+ */
+int ext2fs_super_and_bgd_loc(ext2_filsys fs,
+ dgrp_t group,
+ blk_t *ret_super_blk,
+ blk_t *ret_old_desc_blk,
+ blk_t *ret_new_desc_blk,
+ int *ret_meta_bg)
+{
+ int ret;
+ blk64_t ret_super_blk2;
+ blk64_t ret_old_desc_blk2;
+ blk64_t ret_new_desc_blk2;
+ blk_t numblocks;
+
+ ret = ext2fs_super_and_bgd_loc2(fs, group, &ret_super_blk2,
+ &ret_old_desc_blk2, &ret_new_desc_blk2,
+ ret_meta_bg);
+
+ if (group == fs->group_desc_count-1) {
+ numblocks = (fs->super->s_blocks_count -
+ fs->super->s_first_data_block) %
+ fs->super->s_blocks_per_group;
+ if (!numblocks)
+ numblocks = fs->super->s_blocks_per_group;
+ } else
+ numblocks = fs->super->s_blocks_per_group;
+
+ if (ret_super_blk)
+ *ret_super_blk = (blk_t)ret_super_blk2;
+ if (ret_old_desc_blk)
+ *ret_old_desc_blk = (blk_t)ret_old_desc_blk2;
+ if (ret_new_desc_blk)
+ *ret_new_desc_blk = (blk_t)ret_new_desc_blk2;
+
+ numblocks -= 2 + fs->inode_blocks_per_group + ret;
+
+ return numblocks;
+}
/*
* This function forces out the primary superblock. We need to only
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index ed99901..f06581c 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -707,6 +707,12 @@ extern errcode_t ext2fs_check_desc(ext2_filsys fs);
extern errcode_t ext2fs_close(ext2_filsys fs);
extern errcode_t ext2fs_flush(ext2_filsys fs);
extern int ext2fs_bg_has_super(ext2_filsys fs, int group_block);
+extern int ext2fs_super_and_bgd_loc2(ext2_filsys fs,
+ dgrp_t group,
+ blk64_t *ret_super_blk,
+ blk64_t *ret_old_desc_blk,
+ blk64_t *ret_new_desc_blk,
+ int *ret_meta_bg);
extern int ext2fs_super_and_bgd_loc(ext2_filsys fs,
dgrp_t group,
blk_t *ret_super_blk,
From: Jose R. Santos <[email protected]>
Add 64-bit openfs interface.
Add new ext2fs_descriptor_block_loc2() routine that takes blk64_t as
an input.
Signed-off-by: Jose R. Santos <[email protected]>
--
lib/ext2fs/ext2fs.h | 2 ++
lib/ext2fs/openfs.c | 12 +++++++++---
2 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index f06581c..3ab8bb7 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1060,6 +1060,8 @@ extern errcode_t ext2fs_open2(const char *name, const char *io_options,
int flags, int superblock,
unsigned int block_size, io_manager manager,
ext2_filsys *ret_fs);
+extern blk64_t ext2fs_descriptor_block_loc2(ext2_filsys fs,
+ blk64_t group_block, dgrp_t i);
extern blk_t ext2fs_descriptor_block_loc(ext2_filsys fs, blk_t group_block,
dgrp_t i);
errcode_t ext2fs_get_data_io(ext2_filsys fs, io_channel *old_io);
diff --git a/lib/ext2fs/openfs.c b/lib/ext2fs/openfs.c
index fc54afe..6b365b7 100644
--- a/lib/ext2fs/openfs.c
+++ b/lib/ext2fs/openfs.c
@@ -29,11 +29,12 @@
#include "ext2fs.h"
#include "e2image.h"
-blk_t ext2fs_descriptor_block_loc(ext2_filsys fs, blk_t group_block, dgrp_t i)
+blk64_t ext2fs_descriptor_block_loc2(ext2_filsys fs, blk64_t group_block,
+ dgrp_t i)
{
int bg;
int has_super = 0;
- int ret_blk;
+ blk64_t ret_blk;
if (!(fs->super->s_feature_incompat & EXT2_FEATURE_INCOMPAT_META_BG) ||
(i < fs->super->s_first_meta_bg))
@@ -42,7 +43,7 @@ blk_t ext2fs_descriptor_block_loc(ext2_filsys fs, blk_t group_block, dgrp_t i)
bg = EXT2_DESC_PER_BLOCK(fs->super) * i;
if (ext2fs_bg_has_super(fs, bg))
has_super = 1;
- ret_blk = ext2fs_group_first_block(fs, bg) + has_super;
+ ret_blk = ext2fs_group_first_block2(fs, bg) + has_super;
/*
* If group_block is not the normal value, we're trying to use
* the backup group descriptors and superblock --- so use the
@@ -58,6 +59,11 @@ blk_t ext2fs_descriptor_block_loc(ext2_filsys fs, blk_t group_block, dgrp_t i)
return ret_blk;
}
+blk_t ext2fs_descriptor_block_loc(ext2_filsys fs, blk_t group_block, dgrp_t i)
+{
+ return ext2fs_descriptor_block_loc2(fs, group_block, i);
+}
+
errcode_t ext2fs_open(const char *name, int flags, int superblock,
unsigned int block_size, io_manager manager,
ext2_filsys *ret_fs)
On Fri, May 09, 2008 at 11:39:52AM -0500, Jose R. Santos wrote:
> + /*FIXME: Not blk64_t clean */
Suggestion, please use "FIXME-64" instead of fixme so it's easier to
clean this up later.
> int group = ext2fs_group_of_blk(fs, blk);
Might as well fix this in the initial patch series, earlier --- and
let's *not* make it be an inline function. Yeah, we call
ext2fs_group_of_blk and ext2fs_group_of_ino a fair amount, but #1, on
32-bit machines, the 64 bit arithmemtic makes the function bigger, and
#2, modern CPU's have greatly reduced the cost of procedure
activations.
- Ted
On Fri, May 09, 2008 at 11:40:05AM -0500, Jose R. Santos wrote:
> From: Jose R. Santos <[email protected]>
>
> Add 64-bit ext_attr interface.
>
> Add ext2fs_read_ext_attr2(), ext2fs_write_ext_attr2() and
> ext2fs_adjust_ea_refcount2() that take blk64_t as an input.
Remind me --- did we ever figure out where we are going to store the
high 32 bits of the external extended attribute block? The low 32
bits are stored in i_file_acl, but I don't remember if we decided
where to put the high 32 bits?
- Ted
On Fri, May 09, 2008 at 11:40:12AM -0500, Jose R. Santos wrote:
> From: Jose R. Santos <[email protected]>
>
> Add new blk64_t handling inline functions
>
> Introduce inline fuctions to handle blk64_t and low/high values in
> super blocks and inodes.
If you do this patch first, then you can address some of the FIXME's
earlier in the patch series...
And as I mentioned, we probably want to avoid making them INLINE
functions. It's always easier to inline a function later if we find
that it shows up on profiles for performance reasons, but if we find
out we need to make changes and it is inline already, it's much harder
to deal with the backwards compatibility issues.
- Ted
On Fri, May 09, 2008 at 11:40:19AM -0500, Jose R. Santos wrote:
> From: Jose R. Santos <[email protected]>
>
> Add 64-bit closefs interface.
>
> Add new ext2fs_super_and_bgd_loc2() that returns blk64_t pointers.
> The function now returns the number of blocks used by super block and
> group descriptors since with flex_bg, it can no longer be assumed that
> bitmaps and inode tables still resided within the block group.
This change makes me nervous, because (a) I need to assure myself that
ext2fs_super_and_bgd_loc() is going to always do the right thing, and
(b) the changes to the callers of ext2fs_super_and_bgd_loc2() aren't
also described.
So this requires more thought. One change I would make is to return
the number of blocks via a pointer rather than through a straight
return value. And also, if I recall correctly, nothing is actually
using the ret_meta_bg pointer, so we might be able to drop that in the
_2 version. of the interface.
We should probably include in this patch series the callers of
ext2fs_super_and_bgd_loc() and assure ourselves that the new interface
actually works correctly and is clean for the users before we finalize
the interface change.
- Ted
On Mon, 12 May 2008 11:05:50 -0400
Theodore Tso <[email protected]> wrote:
> On Fri, May 09, 2008 at 11:40:19AM -0500, Jose R. Santos wrote:
> > From: Jose R. Santos <[email protected]>
> >
> > Add 64-bit closefs interface.
> >
> > Add new ext2fs_super_and_bgd_loc2() that returns blk64_t pointers.
> > The function now returns the number of blocks used by super block and
> > group descriptors since with flex_bg, it can no longer be assumed that
> > bitmaps and inode tables still resided within the block group.
>
> This change makes me nervous, because (a) I need to assure myself that
> ext2fs_super_and_bgd_loc() is going to always do the right thing, and
> (b) the changes to the callers of ext2fs_super_and_bgd_loc2() aren't
> also described.
I thought that we concluded that ext2fs_super_and_bgd_loc() would not
do the right thing which is the reason that ext2fs_super_and_bgd_loc2()
returns just the number of groups used by super block and group
descriptors. Right now, ext2fs_super_and_bgd_loc() works the same as
it has before, and the new ext2fs_super_and_bgd_loc2() would do the
right thing here by not assuming inode tables and bitmaps are located
in the block group.
> So this requires more thought. One change I would make is to return
> the number of blocks via a pointer rather than through a straight
> return value. And also, if I recall correctly, nothing is actually
> using the ret_meta_bg pointer, so we might be able to drop that in the
> _2 version. of the interface.
I will make these two changes.
> We should probably include in this patch series the callers of
> ext2fs_super_and_bgd_loc() and assure ourselves that the new interface
> actually works correctly and is clean for the users before we finalize
> the interface change.
Fair enough. I'll add a patch for this.
> - Ted
-JRS
On Mon, 12 May 2008 10:49:03 -0400
Theodore Tso <[email protected]> wrote:
> On Fri, May 09, 2008 at 11:40:12AM -0500, Jose R. Santos wrote:
> > From: Jose R. Santos <[email protected]>
> >
> > Add new blk64_t handling inline functions
> >
> > Introduce inline fuctions to handle blk64_t and low/high values in
> > super blocks and inodes.
>
> If you do this patch first, then you can address some of the FIXME's
> earlier in the patch series...
Will do.
> And as I mentioned, we probably want to avoid making them INLINE
> functions. It's always easier to inline a function later if we find
> that it shows up on profiles for performance reasons, but if we find
> out we need to make changes and it is inline already, it's much harder
> to deal with the backwards compatibility issues.
I agree that making the not inline is the right thing to do, but now
the question is where do we put these new functions. Is it still
appropriate to place them in ext2fs.h or do you prefer a new location?
> - Ted
-JRS
On Mon, 12 May 2008 10:43:53 -0400
Theodore Tso <[email protected]> wrote:
> On Fri, May 09, 2008 at 11:39:52AM -0500, Jose R. Santos wrote:
> > + /*FIXME: Not blk64_t clean */
>
> Suggestion, please use "FIXME-64" instead of fixme so it's easier to
> clean this up later.
Will do.
> > int group = ext2fs_group_of_blk(fs, blk);
Ops... I missed this one.
> Might as well fix this in the initial patch series, earlier --- and
> let's *not* make it be an inline function. Yeah, we call
> ext2fs_group_of_blk and ext2fs_group_of_ino a fair amount, but #1, on
> 32-bit machines, the 64 bit arithmemtic makes the function bigger, and
> #2, modern CPU's have greatly reduced the cost of procedure
> activations.
Will fix it in the series.
> - Ted
-JRS
On Mon, May 12, 2008 at 12:25:21PM -0500, Jose R. Santos wrote:
>
> I agree that making the not inline is the right thing to do, but now
> the question is where do we put these new functions. Is it still
> appropriate to place them in ext2fs.h or do you prefer a new location?
Once they are no longer non-inline functions they obviously shouldnt
go in ext2fs.h. So it would probably have to be a new file, maybe
miscfuncs.c or some such in lib/ext2fs.
- Ted
On Mon, May 12, 2008 at 12:24:43PM -0500, Jose R. Santos wrote:
>
> I thought that we concluded that ext2fs_super_and_bgd_loc() would not
> do the right thing which is the reason that ext2fs_super_and_bgd_loc2()
> returns just the number of groups used by super block and group
> descriptors. Right now, ext2fs_super_and_bgd_loc() works the same as
> it has before, and the new ext2fs_super_and_bgd_loc2() would do the
> right thing here by not assuming inode tables and bitmaps are located
> in the block group.
>
I dealt with the situation by having ext2fs_initialize back out the
inode table accounting, which worked around the problem, and indeed
that's the only place where the numblocks value is even used. So my
point is that if you make the change in ext2fs_super_and_bgd_loc2(),
it will have ripple effects to at least ext2fs_initialize() --- it
would not be safe to just change ext2fs_super_and_bgd_loc to
ext2fs_super_and_bgd_loc2, since you're making semantic change to what
the function actually returns.
So by not including the change in ext2fs_initialize(), and because
ext2fs_initialize() doesn't call ext2fs_super_and_bgd_loc2() directly,
but indirectly through ext2fs_reserve_super_and_bgd(), we're setting
ourselves up to forget to make this change, and thus introduce a bug.
This is *especially* true since ext2fs_reserve_super_and_bgd() would
apparently not need to be changed since it doesn't return a blk_t ---
so it would be a case where the function wouldn't change, but its
behaviour (via its return code) would be changing, which is dangerous.
Practicing defensive programming is a good thing here, which is why I
suggested making it return numblocks via a pointer, and then very
carefully *documenting* what the new parameter contains, and then
documenting the change in ext2fs_reserve_super_and_bgd2(), and then in
turn to ext2fs_initialize(), is just a much safer way to go.
- Ted