diff mbox series

[02/15] ext2fs, e2fsck: add kernel endian-ness conversion macros

Message ID 20201120191606.2224881-3-harshadshirwadkar@gmail.com
State Superseded
Headers show
Series Fast commits support for e2fsprogs | expand

Commit Message

harshad shirwadkar Nov. 20, 2020, 7:15 p.m. UTC
In order to make recovery.c identical with kernel, we need endianness
conversion macros (such as cpu_to_be32 and friends) defined in
e2fsprogs. This patch defines these macros and also fixes recovery.c
to use these. These macros are also needed for fast commit recovery
patches later in this series.

Signed-off-by: Harshad Shirwadkar <harshadshirwadkar@gmail.com>
---
 e2fsck/recovery.c   | 42 ++++++++++--------------------------------
 lib/ext2fs/bitops.h |  8 ++++++++
 2 files changed, 18 insertions(+), 32 deletions(-)

Comments

Theodore Ts'o Dec. 2, 2020, 4:50 p.m. UTC | #1
On Fri, Nov 20, 2020 at 11:15:53AM -0800, Harshad Shirwadkar wrote:
> diff --git a/lib/ext2fs/bitops.h b/lib/ext2fs/bitops.h
> index 505b3c9c..3c7b2496 100644
> --- a/lib/ext2fs/bitops.h
> +++ b/lib/ext2fs/bitops.h
> @@ -247,6 +247,14 @@ extern errcode_t ext2fs_find_first_set_generic_bmap(ext2fs_generic_bitmap bitmap
>  #endif /* __STDC_VERSION__ >= 199901L */
>  #endif /* INCLUDE_INLINE_FUNCS */
>  
> +/* Macros for kernel compatibility */
> +#define be32_to_cpu(x)		ext2fs_be32_to_cpu(x)
> +#define le32_to_cpu(x)		ext2fs_le32_to_cpu(x)
> +#define le16_to_cpu(x)		ext2fs_le16_to_cpu(x)
> +
> +#define cpu_to_be32(x)		ext2fs_cpu_to_be32(x)
> +#define cpu_to_be16(x)		ext2fs_cpu_to_be16(x)
> +#define cpu_to_le16(x)		ext2fs_cpu_to_le16(x)
>  /*
>   * Fast bit set/clear functions that doesn't need to return the
>   * previous bit value.

Kernel compatibility #define's should be in e2fsck/jfs_user.h.

The problem with putting them in lib/ext2fs/bitops.h is that this a
published header file which will be pulled in by external userspace
applications which #include <ext2fs/ext2fs.h>.  And we don't want to
have namespace leakage which might interfere with other header files
or the application's definition of these cpp macros.

						- Ted
harshad shirwadkar Dec. 3, 2020, 6:10 p.m. UTC | #2
Ah makes sense, sorry I missed that entirely in the patch series, I'll
revisit and will be more careful about what goes in libext2fs.

Thanks,
Harshad


On Wed, Dec 2, 2020 at 8:50 AM Theodore Y. Ts'o <tytso@mit.edu> wrote:
>
> On Fri, Nov 20, 2020 at 11:15:53AM -0800, Harshad Shirwadkar wrote:
> > diff --git a/lib/ext2fs/bitops.h b/lib/ext2fs/bitops.h
> > index 505b3c9c..3c7b2496 100644
> > --- a/lib/ext2fs/bitops.h
> > +++ b/lib/ext2fs/bitops.h
> > @@ -247,6 +247,14 @@ extern errcode_t ext2fs_find_first_set_generic_bmap(ext2fs_generic_bitmap bitmap
> >  #endif /* __STDC_VERSION__ >= 199901L */
> >  #endif /* INCLUDE_INLINE_FUNCS */
> >
> > +/* Macros for kernel compatibility */
> > +#define be32_to_cpu(x)               ext2fs_be32_to_cpu(x)
> > +#define le32_to_cpu(x)               ext2fs_le32_to_cpu(x)
> > +#define le16_to_cpu(x)               ext2fs_le16_to_cpu(x)
> > +
> > +#define cpu_to_be32(x)               ext2fs_cpu_to_be32(x)
> > +#define cpu_to_be16(x)               ext2fs_cpu_to_be16(x)
> > +#define cpu_to_le16(x)               ext2fs_cpu_to_le16(x)
> >  /*
> >   * Fast bit set/clear functions that doesn't need to return the
> >   * previous bit value.
>
> Kernel compatibility #define's should be in e2fsck/jfs_user.h.
>
> The problem with putting them in lib/ext2fs/bitops.h is that this a
> published header file which will be pulled in by external userspace
> applications which #include <ext2fs/ext2fs.h>.  And we don't want to
> have namespace leakage which might interfere with other header files
> or the application's definition of these cpp macros.
>
>                                                 - Ted
diff mbox series

Patch

diff --git a/e2fsck/recovery.c b/e2fsck/recovery.c
index 5df690ad..6c3b7bb4 100644
--- a/e2fsck/recovery.c
+++ b/e2fsck/recovery.c
@@ -121,27 +121,6 @@  failed:
 
 #endif /* __KERNEL__ */
 
-static inline __u32 get_be32(__be32 *p)
-{
-	unsigned char *cp = (unsigned char *) p;
-	__u32 ret;
-
-	ret = *cp++;
-	ret = (ret << 8) + *cp++;
-	ret = (ret << 8) + *cp++;
-	ret = (ret << 8) + *cp++;
-	return ret;
-}
-
-static inline __u16 get_be16(__be16 *p)
-{
-	unsigned char *cp = (unsigned char *) p;
-	__u16 ret;
-
-	ret = *cp++;
-	ret = (ret << 8) + *cp++;
-	return ret;
-}
 
 /*
  * Read a block from the journal
@@ -232,10 +211,10 @@  static int count_tags(journal_t *journal, struct buffer_head *bh)
 
 		nr++;
 		tagp += tag_bytes;
-		if (!(get_be16(&tag->t_flags) & JBD2_FLAG_SAME_UUID))
+		if (!(tag->t_flags & cpu_to_be16(JBD2_FLAG_SAME_UUID)))
 			tagp += 16;
 
-		if (get_be16(&tag->t_flags) & JBD2_FLAG_LAST_TAG)
+		if (tag->t_flags & cpu_to_be16(JBD2_FLAG_LAST_TAG))
 			break;
 	}
 
@@ -358,9 +337,9 @@  int jbd2_journal_skip_recovery(journal_t *journal)
 static inline unsigned long long read_tag_block(journal_t *journal,
 						journal_block_tag_t *tag)
 {
-	unsigned long long block = get_be32(&tag->t_blocknr);
+	unsigned long long block = be32_to_cpu(tag->t_blocknr);
 	if (jbd2_has_feature_64bit(journal))
-		block |= (u64)get_be32(&tag->t_blocknr_high) << 32;
+		block |= (u64)be32_to_cpu(tag->t_blocknr_high) << 32;
 	return block;
 }
 
@@ -429,9 +408,9 @@  static int jbd2_block_tag_csum_verify(journal_t *j, journal_block_tag_t *tag,
 	csum32 = jbd2_chksum(j, csum32, buf, j->j_blocksize);
 
 	if (jbd2_has_feature_csum3(j))
-		return get_be32(&tag3->t_checksum) == csum32;
-
-	return get_be16(&tag->t_checksum) == (csum32 & 0xFFFF);
+		return tag3->t_checksum == cpu_to_be32(csum32);
+	else
+		return tag->t_checksum == cpu_to_be16(csum32);
 }
 
 static int do_one_pass(journal_t *journal,
@@ -579,7 +558,7 @@  static int do_one_pass(journal_t *journal,
 				unsigned long io_block;
 
 				tag = (journal_block_tag_t *) tagp;
-				flags = get_be16(&tag->t_flags);
+				flags = be16_to_cpu(tag->t_flags);
 
 				io_block = next_log_block++;
 				wrap(journal, next_log_block);
@@ -643,9 +622,8 @@  static int do_one_pass(journal_t *journal,
 					memcpy(nbh->b_data, obh->b_data,
 							journal->j_blocksize);
 					if (flags & JBD2_FLAG_ESCAPE) {
-						__be32 magic = cpu_to_be32(JBD2_MAGIC_NUMBER);
-						memcpy(nbh->b_data, &magic,
-						       sizeof(magic));
+						*((__be32 *)nbh->b_data) =
+						cpu_to_be32(JBD2_MAGIC_NUMBER);
 					}
 
 					BUFFER_TRACE(nbh, "marking dirty");
diff --git a/lib/ext2fs/bitops.h b/lib/ext2fs/bitops.h
index 505b3c9c..3c7b2496 100644
--- a/lib/ext2fs/bitops.h
+++ b/lib/ext2fs/bitops.h
@@ -247,6 +247,14 @@  extern errcode_t ext2fs_find_first_set_generic_bmap(ext2fs_generic_bitmap bitmap
 #endif /* __STDC_VERSION__ >= 199901L */
 #endif /* INCLUDE_INLINE_FUNCS */
 
+/* Macros for kernel compatibility */
+#define be32_to_cpu(x)		ext2fs_be32_to_cpu(x)
+#define le32_to_cpu(x)		ext2fs_le32_to_cpu(x)
+#define le16_to_cpu(x)		ext2fs_le16_to_cpu(x)
+
+#define cpu_to_be32(x)		ext2fs_cpu_to_be32(x)
+#define cpu_to_be16(x)		ext2fs_cpu_to_be16(x)
+#define cpu_to_le16(x)		ext2fs_cpu_to_le16(x)
 /*
  * Fast bit set/clear functions that doesn't need to return the
  * previous bit value.