Skip to contentSkip to conversation

UDF - use UDF_MAX_WRITE_VERSION instead of numbers

PATCH In review Cyrill Gorcunov 2 participants 5 messages Apr 12, 2008 – Apr 13, 2008 UDF filesystem

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 UDF - use UDF_MAX_WRITE_VERSION instead of numbers

---

Jan, the patch is over current yours for_mm branch

Yep, i know it exceeds 80 column *but* it looks much better in this way ;)

fs/udf/inode.c +5 -5
Index: linux-2.6.git/fs/udf/inode.c
===================================================================
@@ -1732,7 +1732,7 @@ int8_t udf_add_aext(struct inode *inode,
 		}
 		if (epos->bh) {
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 				udf_update_tag(epos->bh->b_data, loffset);
 			else
 				udf_update_tag(epos->bh->b_data,
@@ -1754,7 +1754,7 @@ int8_t udf_add_aext(struct inode *inode,
 		aed = (struct allocExtDesc *)epos->bh->b_data;
 		le32_add_cpu(&aed->lengthAllocDescs, adsize);
 		if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-				UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+				UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 			udf_update_tag(epos->bh->b_data,
 					epos->offset + (inc ? 0 : adsize));
 		else
@@ -1802,7 +1802,7 @@ int8_t udf_write_aext(struct inode *inod
 
 	if (epos->bh) {
 		if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-		    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201) {
+		    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION) {
 			struct allocExtDesc *aed =
 				(struct allocExtDesc *)epos->bh->b_data;
 			udf_update_tag(epos->bh->b_data,
@@ -1968,7 +1968,7 @@ int8_t udf_delete_aext(struct inode *ino
 			aed = (struct allocExtDesc *)oepos.bh->b_data;
 			le32_add_cpu(&aed->lengthAllocDescs, -(2 * adsize));
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 				udf_update_tag(oepos.bh->b_data,
 						oepos.offset - (2 * adsize));
 			else
@@ -1985,7 +1985,7 @@ int8_t udf_delete_aext(struct inode *ino
 			aed = (struct allocExtDesc *)oepos.bh->b_data;
 			le32_add_cpu(&aed->lengthAllocDescs, -adsize);
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 				udf_update_tag(oepos.bh->b_data,
 						epos.offset - adsize);
 			else
fs/udf/truncate.c +2 -2
Index: linux-2.6.git/fs/udf/truncate.c
===================================================================
@@ -165,7 +165,7 @@ void udf_discard_prealloc(struct inode *
 				cpu_to_le32(epos.offset -
 					    sizeof(struct allocExtDesc));
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 				udf_update_tag(epos.bh->b_data, epos.offset);
 			else
 				udf_update_tag(epos.bh->b_data,
@@ -190,7 +190,7 @@ static void udf_update_alloc_ext_desc(st
 	int len = sizeof(struct allocExtDesc);
 
 	aed->lengthAllocDescs =	cpu_to_le32(lenalloc);
-	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT) || sbi->s_udfrev >= 0x0201)
+	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_STRICT) || sbi->s_udfrev >= UDF_MAX_WRITE_VERSION)
 		len += lenalloc;
 
 	udf_update_tag(epos->bh->b_data, len);
#1 lore raw
Quoted 13 lines · Cyrill Gorcunov

---

Jan, the patch is over current yours for_mm branch

Yep, i know it exceeds 80 column *but* it looks much better in this way ;)

Index: linux-2.6.git/fs/udf/inode.c
===================================================================

fs/udf/inode.c int8_t udf_add_aext(struct inode *inode, · line 1738

Quoted 8 lines
fs/udf/inode.c +1 -1
@@ -1732,7 +1732,7 @@ int8_t udf_add_aext(struct inode *inode,
 		}
 		if (epos->bh) {
 			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
-			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
+			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
 				udf_update_tag(epos->bh->b_data, loffset);
 			else
 				udf_update_tag(epos->bh->b_data,

I think this patch is wrong. Right now it doesn't change anything, but in future when someone will add support for writing UDF > 2.01 (and bump UDF_MAX_WRITE_VERSION) it will break for filesystems written with udfrev >= 2.01 && udfrev < UDF_MAX_WRITE_VERSION.

Marcin

#2 lore raw

[Marcin Slusarz - Sun, Apr 13, 2008 at 01:50:29PM +0200]

| On Sat, Apr 12, 2008 at 11:40:08PM +0400, Cyrill Gorcunov wrote:
| > Signed-off-by: Cyrill Gorcunov <[email protected]>
| > ---
| > 
| > Jan, the patch is over current yours for_mm branch
| > 
| > Yep, i know it exceeds 80 column *but* it looks much better
| > in this way ;)
| > 
| > Index: linux-2.6.git/fs/udf/inode.c
| > ===================================================================
| > --- linux-2.6.git.orig/fs/udf/inode.c	2008-04-12 22:53:15.000000000 +0400
| > +++ linux-2.6.git/fs/udf/inode.c	2008-04-12 23:34:28.000000000 +0400
| > @@ -1732,7 +1732,7 @@ int8_t udf_add_aext(struct inode *inode,
| >  		}
| >  		if (epos->bh) {
| >  			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
| > -			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
| > +			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
| >  				udf_update_tag(epos->bh->b_data, loffset);
| >  			else
| >  				udf_update_tag(epos->bh->b_data,
| I think this patch is wrong. Right now it doesn't change anything, but in future
| when someone will add support for writing UDF > 2.01 (and bump UDF_MAX_WRITE_VERSION)
| it will break for filesystems written with udfrev >= 2.01 && udfrev < UDF_MAX_WRITE_VERSION.
| 
| Marcin
| 

well, if someone add support the writting UDF > 2.01 it will require additional switches/analisys anyway and saving these hard-coded-numbers would not help.

- Cyrill -

#3 lore raw
Quoted 30 lines · Cyrill Gorcunov

[Marcin Slusarz - Sun, Apr 13, 2008 at 01:50:29PM +0200]

| On Sat, Apr 12, 2008 at 11:40:08PM +0400, Cyrill Gorcunov wrote:
| > Signed-off-by: Cyrill Gorcunov <[email protected]>
| > ---
| > 
| > Jan, the patch is over current yours for_mm branch
| > 
| > Yep, i know it exceeds 80 column *but* it looks much better
| > in this way ;)
| > 
| > Index: linux-2.6.git/fs/udf/inode.c
| > ===================================================================
| > --- linux-2.6.git.orig/fs/udf/inode.c	2008-04-12 22:53:15.000000000 +0400
| > +++ linux-2.6.git/fs/udf/inode.c	2008-04-12 23:34:28.000000000 +0400
| > @@ -1732,7 +1732,7 @@ int8_t udf_add_aext(struct inode *inode,
| >  		}
| >  		if (epos->bh) {
| >  			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
| > -			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
| > +			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
| >  				udf_update_tag(epos->bh->b_data, loffset);
| >  			else
| >  				udf_update_tag(epos->bh->b_data,
| I think this patch is wrong. Right now it doesn't change anything, but in future
| when someone will add support for writing UDF > 2.01 (and bump UDF_MAX_WRITE_VERSION)
| it will break for filesystems written with udfrev >= 2.01 && udfrev < UDF_MAX_WRITE_VERSION.
| 
| Marcin
| 
Quoted 3 lines

well, if someone add support the writting UDF > 2.01 it will require additional switches/analisys anyway and saving these hard-coded-numbers would not help.

Yes, but these values don't correlate with UDF_MAX_WRITE_VERSION - it's simple coincidence. If you really don't like these numbers add another constant.

Marcin

#4 lore raw

[Marcin Slusarz - Sun, Apr 13, 2008 at 03:56:10PM +0200]

| On Sun, Apr 13, 2008 at 04:06:22PM +0400, Cyrill Gorcunov wrote:
| > [Marcin Slusarz - Sun, Apr 13, 2008 at 01:50:29PM +0200]
| > | On Sat, Apr 12, 2008 at 11:40:08PM +0400, Cyrill Gorcunov wrote:
| > | > Signed-off-by: Cyrill Gorcunov <[email protected]>
| > | > ---
| > | > 
| > | > Jan, the patch is over current yours for_mm branch
| > | > 
| > | > Yep, i know it exceeds 80 column *but* it looks much better
| > | > in this way ;)
| > | > 
| > | > Index: linux-2.6.git/fs/udf/inode.c
| > | > ===================================================================
| > | > --- linux-2.6.git.orig/fs/udf/inode.c	2008-04-12 22:53:15.000000000 +0400
| > | > +++ linux-2.6.git/fs/udf/inode.c	2008-04-12 23:34:28.000000000 +0400
| > | > @@ -1732,7 +1732,7 @@ int8_t udf_add_aext(struct inode *inode,
| > | >  		}
| > | >  		if (epos->bh) {
| > | >  			if (!UDF_QUERY_FLAG(inode->i_sb, UDF_FLAG_STRICT) ||
| > | > -			    UDF_SB(inode->i_sb)->s_udfrev >= 0x0201)
| > | > +			    UDF_SB(inode->i_sb)->s_udfrev >= UDF_MAX_WRITE_VERSION)
| > | >  				udf_update_tag(epos->bh->b_data, loffset);
| > | >  			else
| > | >  				udf_update_tag(epos->bh->b_data,
| > | I think this patch is wrong. Right now it doesn't change anything, but in future
| > | when someone will add support for writing UDF > 2.01 (and bump UDF_MAX_WRITE_VERSION)
| > | it will break for filesystems written with udfrev >= 2.01 && udfrev < UDF_MAX_WRITE_VERSION.
| > | 
| > | Marcin
| > | 
| > 
| > well, if someone add support the writting UDF > 2.01 it will require
| > additional switches/analisys anyway and saving these hard-coded-numbers
| > would not help.
| 
| Yes, but these values don't correlate with UDF_MAX_WRITE_VERSION - it's
| simple coincidence. If you really don't like these numbers add another
| constant.
| 
| Marcin
| 

Ugh! You're right, thanks ;) Jan, drop them, sorry

- Cyrill -