Dave Kleikamp wrote:
>>> Against 2.4.20-pre5 - fix up the type of nlink_t. This makes jfs and
>>> reiserfs stop complaining about comparisons always turning up false
>>> due to limited range of data type.
>>>
>>>If you change this, you change the types exported to userspace
>>>which will break everything.
>>>
>>>
>>Right. Here's a corresponding reiserfs/jfs fix, then. I've checked the
>>constants aren't used for anything else except nlink overflow alerts.
>>
>>
>
>I don't like this fix. I know 32767 is a lot of links, but I don't like
>artificially lowering a limit like this just because one architecture
>defines nlink_t incorrectly. I'd rather get rid of the compiler warnings
>with a cast in the few places the limit is checked, even though that is
>a little bit ugly.
>
>
>
>>diff -urN linux-2.4.20-pre5/fs/jfs/jfs_filsys.h linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h
>>--- linux-2.4.20-pre5/fs/jfs/jfs_filsys.h 2002-09-01 11:31:44.000000000 +0200
>>+++ linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h 2002-09-01 11:30:13.000000000 +0200
>>@@ -125,7 +125,8 @@
>> #define MAXBLOCKSIZE 4096
>> #define MAXFILESIZE ((s64)1 << 52)
>>
>>-#define JFS_LINK_MAX 65535 /* nlink_t is unsigned short */
>>+/* the shortest nlink_t there is is sparc's signed short */
>>+#define JFS_LINK_MAX 32767
>>
>> /* Minimum number of bytes supported for a JFS partition */
>> #define MINJFS (0x1000000)
>>diff -urN linux-2.4.20-pre5/include/linux/reiserfs_fs.h linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h
>>--- linux-2.4.20-pre5/include/linux/reiserfs_fs.h 2002-09-01 11:31:45.000000000 +0200
>>+++ linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h 2002-09-01 11:23:30.000000000 +0200
>>@@ -1185,10 +1185,12 @@
>> #define MAX_B_NUM MAX_UL_INT
>> #define MAX_FC_NUM MAX_US_INT
>>
>>-
>>-/* the purpose is to detect overflow of an unsigned short */
>>-#define REISERFS_LINK_MAX (MAX_US_INT - 1000)
>>-
>>+/* the original purpose was to detect a possible overflow
>>+ * of an unsigned short nlink_t. However, there are archs
>>+ * that only provide a signed short nlink_t, so this will
>>+ * have to start ringing a wee bit earlier.
>>+ */
>>+#define REISERFS_LINK_MAX (0x7fff - 1000)
>>
>> /* The following defines are used in reiserfs_insert_item and reiserfs_append_item */
>> #define REISERFS_KERNEL_MEM 0 /* reiserfs kernel memory mode */
>>
>>
>
>Thanks,
>Shaggy
>
>
>
I think you are sort of right. You are right to dislike this patch for
the reasons you state. The proper fix should be to make the result of
the limit computation be accurately architecture specific. I wasn't
paying enough attention --- hardcoding
0x7fff - 1000
is ugly. We need to find some appropriate #define to subtract 1000 from.
Green, please scan the code for the magic constant to compute from, and
code something unless Dave or someone does it before you.
> >>> Against 2.4.20-pre5 - fix up the type of nlink_t. This makes jfs and
> >>> reiserfs stop complaining about comparisons always turning up false
> >>> due to limited range of data type.
> >>>
> >>>If you change this, you change the types exported to userspace
> >>>which will break everything.
> >>>
> >>>
> >>Right. Here's a corresponding reiserfs/jfs fix, then. I've checked the
> >>constants aren't used for anything else except nlink overflow alerts.
> >>
> >>
> >
> >I don't like this fix. I know 32767 is a lot of links, but I don't like
> >artificially lowering a limit like this just because one architecture
> >defines nlink_t incorrectly. I'd rather get rid of the compiler warnings
> >with a cast in the few places the limit is checked, even though that is
> >a little bit ugly.
> >
> >
> >
> >>diff -urN linux-2.4.20-pre5/fs/jfs/jfs_filsys.h
> >>linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h
> >>--- linux-2.4.20-pre5/fs/jfs/jfs_filsys.h 2002-09-01
> >>11:31:44.000000000 +0200
> >>+++ linux-2.4.20-pre5.n/fs/jfs/jfs_filsys.h 2002-09-01
> >>11:30:13.000000000 +0200
> >>@@ -125,7 +125,8 @@
> >>#define MAXBLOCKSIZE 4096
> >>#define MAXFILESIZE ((s64)1 << 52)
> >>
> >>-#define JFS_LINK_MAX 65535 /* nlink_t is unsigned short
> >>*/
> >>+/* the shortest nlink_t there is is sparc's signed short */
> >>+#define JFS_LINK_MAX 32767
> >>
> >>/* Minimum number of bytes supported for a JFS partition */
> >>#define MINJFS (0x1000000)
> >>diff -urN linux-2.4.20-pre5/include/linux/reiserfs_fs.h
> >>linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h
> >>--- linux-2.4.20-pre5/include/linux/reiserfs_fs.h 2002-09-01
> >>11:31:45.000000000 +0200
> >>+++ linux-2.4.20-pre5.n/include/linux/reiserfs_fs.h 2002-09-01
> >>11:23:30.000000000 +0200
> >>@@ -1185,10 +1185,12 @@
> >>#define MAX_B_NUM MAX_UL_INT
> >>#define MAX_FC_NUM MAX_US_INT
> >>
> >>-
> >>-/* the purpose is to detect overflow of an unsigned short */
> >>-#define REISERFS_LINK_MAX (MAX_US_INT - 1000)
> >>-
> >>+/* the original purpose was to detect a possible overflow
> >>+ * of an unsigned short nlink_t. However, there are archs
> >>+ * that only provide a signed short nlink_t, so this will
> >>+ * have to start ringing a wee bit earlier.
> >>+ */
> >>+#define REISERFS_LINK_MAX (0x7fff - 1000)
> >>
> >>/* The following defines are used in reiserfs_insert_item and
> >>reiserfs_append_item */
> >>#define REISERFS_KERNEL_MEM 0 /* reiserfs kernel memory
> >>mode */
> >>
> >>
> >
> >Thanks,
> >Shaggy
> >
> >
> >
> I think you are sort of right. You are right to dislike this patch for
> the reasons you state. The proper fix should be to make the result of
> the limit computation be accurately architecture specific. I wasn't
> paying enough attention --- hardcoding
>
> 0x7fff - 1000
>
> is ugly. We need to find some appropriate #define to subtract 1000 from.
>
> Green, please scan the code for the magic constant to compute from, and
> code something unless Dave or someone does it before you.
All right, how about this? ->
typedef unsigned long long u64;
/* usable for char, short int, and int */
#define set_to_max(a) \
{ \
u64 max = ((u64) 2 << (sizeof(a) * 8 - 1)) - 1; \
a = max; if ((u64) a != max) a = max / 2; \
}
Tomas
Hi,
On Wed, 4 Sep 2002, Tomas Szepe wrote:
> typedef unsigned long long u64;
>
> /* usable for char, short int, and int */
> #define set_to_max(a) \
> { \
> u64 max = ((u64) 2 << (sizeof(a) * 8 - 1)) - 1; \
> a = max; if ((u64) a != max) a = max / 2; \
> }
To make it more secure, we should consider the following version:
typedef unsigned long long u64;
/* usable for char, short int, and int */
#define set_to_max(a) { \
u64 __val_max = ((u64) 1 << (sizeof(a) * 8)) - 1; \
a = __val_max; \
if ((u64) a != __val_max) \
a = __val_max / 2; \
}
So it's basically naming.
Thunder
--
--./../...-/. -.--/---/..-/.-./..././.-../..-. .---/..-/.../- .-
--/../-./..-/-/./--..-- ../.----./.-../.-.. --./../...-/. -.--/---/..-
.- -/---/--/---/.-./.-./---/.--/.-.-.-
--./.-/-.../.-./.././.-../.-.-.-
Hi,
On Wed, 4 Sep 2002, Thunder from the hill wrote:
> On Wed, 4 Sep 2002, Tomas Szepe wrote:
> > typedef unsigned long long u64;
> >
> > /* usable for char, short int, and int */
> > #define set_to_max(a) \
> > { \
> > u64 max = ((u64) 2 << (sizeof(a) * 8 - 1)) - 1; \
> > a = max; if ((u64) a != max) a = max / 2; \
> > }
This is the wholesale:
typedef unsigned long long u64;
/* usable for char, short int, and int */
#define set_to_max(a) do { \
u64 __val_max = ((u64) 1 << (sizeof(a) * 8)) - 1; \
a = __val_max; \
if ((u64) a != __val_max) \
a = __val_max / 2; \
} while(0)
#define set_to_min(a) do { \
set_to_max(a); \
a++; \
} while(0)
Thunder
--
--./../...-/. -.--/---/..-/.-./..././.-../..-. .---/..-/.../- .-
--/../-./..-/-/./--..-- ../.----./.-../.-.. --./../...-/. -.--/---/..-
.- -/---/--/---/.-./.-./---/.--/.-.-.-
--./.-/-.../.-./.././.-../.-.-.-
From: Hans Reiser <[email protected]>
Date: Thu, 05 Sep 2002 00:31:36 +0400
The proper fix should be to make the result of the limit
computation be accurately architecture specific.
And then each and every Reiserfs partition is platform specific
and cannot be mounted onto another Linux platform.
Creating such a restriction is a grave error.