[PATCH v3 2/2] staging: erofs: Modify conditional checks

[PATCH v3 2/2] staging: erofs: Modify conditional checks

[Bhagyashri P. Dighole]

Fix coding style alignment issues detected by checkpatch.pl
Use ! for NULL test rather than explicitly comparing to NULL.

Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
---
 drivers/staging/erofs/data.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index b66191ad44aa..5a55f0bfdfbb 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -53,7 +53,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 
 repeat:
 	page = find_or_create_page(mapping, blkaddr, gfp);
-	if (unlikely(page == NULL)) {
+	if (unlikely(!page)) {
 		DBG_BUGON(nofail);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -177,7 +177,7 @@ int erofs_map_blocks_iter(struct inode *inode,
 {
 	/* by default, reading raw data never use erofs_map_blocks_iter */
 	if (unlikely(!is_inode_layout_compression(inode))) {
-		if (*mpage_ret != NULL)
+		if (*mpage_ret)
 			put_page(*mpage_ret);
 		*mpage_ret = NULL;
 
@@ -200,7 +200,7 @@ int erofs_map_blocks(struct inode *inode,
 		int err;
 
 		err = erofs_map_blocks_iter(inode, map, &mpage, flags);
-		if (mpage != NULL)
+		if (mpage)
 			put_page(mpage);
 		return err;
 	}
@@ -232,7 +232,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	}
 
 	/* note that for readpage case, bio also equals to NULL */
-	if (bio != NULL &&
+	if (bio &&
 	    /* not continuous */
 	    *last_block + 1 != current_block) {
 submit_bio_retry:
@@ -240,7 +240,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 		bio = NULL;
 	}
 
-	if (bio == NULL) {
+	if (!bio) {
 		struct erofs_map_blocks map = {
 			.m_la = blknr_to_addr(current_block),
 		};
@@ -342,7 +342,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	unlock_page(page);
 
 	/* if updated manually, continuous pages has a gap */
-	if (bio != NULL)
+	if (bio)
 submit_bio_out:
 		__submit_bio(bio, REQ_OP_READ, 0);
 
@@ -408,7 +408,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 	DBG_BUGON(!list_empty(pages));
 
 	/* the rare case (end in gaps) */
-	if (unlikely(bio != NULL))
+	if (unlikely(bio))
 		__submit_bio(bio, REQ_OP_READ, 0);
 	return 0;
 }
-- 
2.19.1

[PATCH v3 2/2] staging: erofs: Modify conditional checks

[Bhagyashri P. Dighole]

Fix coding style alignment issues detected by checkpatch.pl
Use ! for NULL test rather than explicitly comparing to NULL.

Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
---
 drivers/staging/erofs/data.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index b66191ad44aa..5a55f0bfdfbb 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -53,7 +53,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 
 repeat:
 	page = find_or_create_page(mapping, blkaddr, gfp);
-	if (unlikely(page == NULL)) {
+	if (unlikely(!page)) {
 		DBG_BUGON(nofail);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -177,7 +177,7 @@ int erofs_map_blocks_iter(struct inode *inode,
 {
 	/* by default, reading raw data never use erofs_map_blocks_iter */
 	if (unlikely(!is_inode_layout_compression(inode))) {
-		if (*mpage_ret != NULL)
+		if (*mpage_ret)
 			put_page(*mpage_ret);
 		*mpage_ret = NULL;
 
@@ -200,7 +200,7 @@ int erofs_map_blocks(struct inode *inode,
 		int err;
 
 		err = erofs_map_blocks_iter(inode, map, &mpage, flags);
-		if (mpage != NULL)
+		if (mpage)
 			put_page(mpage);
 		return err;
 	}
@@ -232,7 +232,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	}
 
 	/* note that for readpage case, bio also equals to NULL */
-	if (bio != NULL &&
+	if (bio &&
 	    /* not continuous */
 	    *last_block + 1 != current_block) {
 submit_bio_retry:
@@ -240,7 +240,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 		bio = NULL;
 	}
 
-	if (bio == NULL) {
+	if (!bio) {
 		struct erofs_map_blocks map = {
 			.m_la = blknr_to_addr(current_block),
 		};
@@ -342,7 +342,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	unlock_page(page);
 
 	/* if updated manually, continuous pages has a gap */
-	if (bio != NULL)
+	if (bio)
 submit_bio_out:
 		__submit_bio(bio, REQ_OP_READ, 0);
 
@@ -408,7 +408,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 	DBG_BUGON(!list_empty(pages));
 
 	/* the rare case (end in gaps) */
-	if (unlikely(bio != NULL))
+	if (unlikely(bio))
 		__submit_bio(bio, REQ_OP_READ, 0);
 	return 0;
 }
-- 
2.19.1

[PATCH v3 2/2] staging: erofs: Modify conditional checks

[Greg Kroah-Hartman]

On Mon, Nov 05, 2018@12:35:03PM +0000, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Use ! for NULL test rather than explicitly comparing to NULL.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
> ---
>  drivers/staging/erofs/data.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Why did you send this patch twice?

And what changed in it from v1 and v2?  Always put that information
below the --- line.

Please fix this up and resend the whole series with that information.

thanks,

greg k-h

Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Greg Kroah-Hartman]

On Mon, Nov 05, 2018 at 12:35:03PM +0000, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Use ! for NULL test rather than explicitly comparing to NULL.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
> ---
>  drivers/staging/erofs/data.c | 14 +++++++-------
>  1 file changed, 7 insertions(+), 7 deletions(-)

Why did you send this patch twice?

And what changed in it from v1 and v2?  Always put that information
below the --- line.

Please fix this up and resend the whole series with that information.

thanks,

greg k-h

[PATCH v3 1/2] staging: erofs: Modify coding style alignments

[Bhagyashri P. Dighole]

Fix coding style alignment issues detected by checkpatch.pl
Matched alignments with parenthesis.

Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
---
Changes in v3:
- Split patch in two patches. Each patch have similar 
  kind of changes. One patch for all alignment fixes and 
  second patch for all conditional check fixes.

Changes in v2:
- Fix for one alignment issue and one conditional check issue.

Changes in v1:
- Fix for all issues in data.c

 drivers/staging/erofs/data.c | 51 ++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index 6384f73e5418..b66191ad44aa 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -40,7 +40,7 @@ static inline void read_endio(struct bio *bio)
 
 /* prio -- true is used for dir */
 struct page *__erofs_get_meta_page(struct super_block *sb,
-	erofs_blk_t blkaddr, bool prio, bool nofail)
+				   erofs_blk_t blkaddr, bool prio, bool nofail)
 {
 	struct inode *const bd_inode = sb->s_bdev->bd_inode;
 	struct address_space *const mapping = bd_inode->i_mapping;
@@ -76,7 +76,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 		}
 
 		__submit_bio(bio, REQ_OP_READ,
-			REQ_META | (prio ? REQ_PRIO : 0));
+			     REQ_META | (prio ? REQ_PRIO : 0));
 
 		lock_page(page);
 
@@ -107,8 +107,8 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 }
 
 static int erofs_map_blocks_flatmode(struct inode *inode,
-	struct erofs_map_blocks *map,
-	int flags)
+				     struct erofs_map_blocks *map,
+				     int flags)
 {
 	int err = 0;
 	erofs_blk_t nblocks, lastblk;
@@ -151,7 +151,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 		map->m_flags |= EROFS_MAP_META;
 	} else {
 		errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
-			vi->nid, inode->i_size, map->m_la);
+		      vi->nid, inode->i_size, map->m_la);
 		DBG_BUGON(1);
 		err = -EIO;
 		goto err_out;
@@ -167,12 +167,13 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 
 #ifdef CONFIG_EROFS_FS_ZIP
 extern int z_erofs_map_blocks_iter(struct inode *,
-	struct erofs_map_blocks *, struct page **, int);
+				   struct erofs_map_blocks *,
+				   struct page **, int);
 #endif
 
 int erofs_map_blocks_iter(struct inode *inode,
-	struct erofs_map_blocks *map,
-	struct page **mpage_ret, int flags)
+			  struct erofs_map_blocks *map,
+			  struct page **mpage_ret, int flags)
 {
 	/* by default, reading raw data never use erofs_map_blocks_iter */
 	if (unlikely(!is_inode_layout_compression(inode))) {
@@ -192,7 +193,7 @@ int erofs_map_blocks_iter(struct inode *inode,
 }
 
 int erofs_map_blocks(struct inode *inode,
-	struct erofs_map_blocks *map, int flags)
+		     struct erofs_map_blocks *map, int flags)
 {
 	if (unlikely(is_inode_layout_compression(inode))) {
 		struct page *mpage = NULL;
@@ -206,13 +207,12 @@ int erofs_map_blocks(struct inode *inode,
 	return erofs_map_blocks_flatmode(inode, map, flags);
 }
 
-static inline struct bio *erofs_read_raw_page(
-	struct bio *bio,
-	struct address_space *mapping,
-	struct page *page,
-	erofs_off_t *last_block,
-	unsigned int nblocks,
-	bool ra)
+static inline struct bio *erofs_read_raw_page(struct bio *bio,
+					      struct address_space *mapping,
+					      struct page *page,
+					      erofs_off_t *last_block,
+					      unsigned int nblocks,
+					      bool ra)
 {
 	struct inode *inode = mapping->host;
 	erofs_off_t current_block = (erofs_off_t)page->index;
@@ -233,8 +233,8 @@ static inline struct bio *erofs_read_raw_page(
 
 	/* note that for readpage case, bio also equals to NULL */
 	if (bio != NULL &&
-		/* not continuous */
-		*last_block + 1 != current_block) {
+	    /* not continuous */
+	    *last_block + 1 != current_block) {
 submit_bio_retry:
 		__submit_bio(bio, REQ_OP_READ, 0);
 		bio = NULL;
@@ -307,7 +307,7 @@ static inline struct bio *erofs_read_raw_page(
 			nblocks = BIO_MAX_PAGES;
 
 		bio = erofs_grab_bio(inode->i_sb,
-			blknr, nblocks, read_endio, false);
+				     blknr, nblocks, read_endio, false);
 
 		if (IS_ERR(bio)) {
 			err = PTR_ERR(bio);
@@ -361,7 +361,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 	trace_erofs_readpage(page, true);
 
 	bio = erofs_read_raw_page(NULL, page->mapping,
-		page, &last_block, 1, false);
+				  page, &last_block, 1, false);
 
 	if (IS_ERR(bio))
 		return PTR_ERR(bio);
@@ -371,8 +371,9 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 }
 
 static int erofs_raw_access_readpages(struct file *filp,
-	struct address_space *mapping,
-	struct list_head *pages, unsigned int nr_pages)
+				      struct address_space *mapping,
+				      struct list_head *pages,
+				      unsigned int nr_pages)
 {
 	erofs_off_t last_block;
 	struct bio *bio = NULL;
@@ -389,13 +390,13 @@ static int erofs_raw_access_readpages(struct file *filp,
 
 		if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) {
 			bio = erofs_read_raw_page(bio, mapping, page,
-				&last_block, nr_pages, true);
+						  &last_block, nr_pages, true);
 
 			/* all the page errors are ignored when readahead */
 			if (IS_ERR(bio)) {
 				pr_err("%s, readahead error at page %lu of nid %llu\n",
-					__func__, page->index,
-					EROFS_V(mapping->host)->nid);
+				       __func__, page->index,
+				       EROFS_V(mapping->host)->nid);
 
 				bio = NULL;
 			}
-- 
2.19.1

[PATCH v3 1/2] staging: erofs: Modify coding style alignments

[Bhagyashri P. Dighole]

Fix coding style alignment issues detected by checkpatch.pl
Matched alignments with parenthesis.

Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
---
Changes in v3:
- Split patch in two patches. Each patch have similar 
  kind of changes. One patch for all alignment fixes and 
  second patch for all conditional check fixes.

Changes in v2:
- Fix for one alignment issue and one conditional check issue.

Changes in v1:
- Fix for all issues in data.c

 drivers/staging/erofs/data.c | 51 ++++++++++++++++++------------------
 1 file changed, 26 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index 6384f73e5418..b66191ad44aa 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -40,7 +40,7 @@ static inline void read_endio(struct bio *bio)
 
 /* prio -- true is used for dir */
 struct page *__erofs_get_meta_page(struct super_block *sb,
-	erofs_blk_t blkaddr, bool prio, bool nofail)
+				   erofs_blk_t blkaddr, bool prio, bool nofail)
 {
 	struct inode *const bd_inode = sb->s_bdev->bd_inode;
 	struct address_space *const mapping = bd_inode->i_mapping;
@@ -76,7 +76,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 		}
 
 		__submit_bio(bio, REQ_OP_READ,
-			REQ_META | (prio ? REQ_PRIO : 0));
+			     REQ_META | (prio ? REQ_PRIO : 0));
 
 		lock_page(page);
 
@@ -107,8 +107,8 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 }
 
 static int erofs_map_blocks_flatmode(struct inode *inode,
-	struct erofs_map_blocks *map,
-	int flags)
+				     struct erofs_map_blocks *map,
+				     int flags)
 {
 	int err = 0;
 	erofs_blk_t nblocks, lastblk;
@@ -151,7 +151,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 		map->m_flags |= EROFS_MAP_META;
 	} else {
 		errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
-			vi->nid, inode->i_size, map->m_la);
+		      vi->nid, inode->i_size, map->m_la);
 		DBG_BUGON(1);
 		err = -EIO;
 		goto err_out;
@@ -167,12 +167,13 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
 
 #ifdef CONFIG_EROFS_FS_ZIP
 extern int z_erofs_map_blocks_iter(struct inode *,
-	struct erofs_map_blocks *, struct page **, int);
+				   struct erofs_map_blocks *,
+				   struct page **, int);
 #endif
 
 int erofs_map_blocks_iter(struct inode *inode,
-	struct erofs_map_blocks *map,
-	struct page **mpage_ret, int flags)
+			  struct erofs_map_blocks *map,
+			  struct page **mpage_ret, int flags)
 {
 	/* by default, reading raw data never use erofs_map_blocks_iter */
 	if (unlikely(!is_inode_layout_compression(inode))) {
@@ -192,7 +193,7 @@ int erofs_map_blocks_iter(struct inode *inode,
 }
 
 int erofs_map_blocks(struct inode *inode,
-	struct erofs_map_blocks *map, int flags)
+		     struct erofs_map_blocks *map, int flags)
 {
 	if (unlikely(is_inode_layout_compression(inode))) {
 		struct page *mpage = NULL;
@@ -206,13 +207,12 @@ int erofs_map_blocks(struct inode *inode,
 	return erofs_map_blocks_flatmode(inode, map, flags);
 }
 
-static inline struct bio *erofs_read_raw_page(
-	struct bio *bio,
-	struct address_space *mapping,
-	struct page *page,
-	erofs_off_t *last_block,
-	unsigned int nblocks,
-	bool ra)
+static inline struct bio *erofs_read_raw_page(struct bio *bio,
+					      struct address_space *mapping,
+					      struct page *page,
+					      erofs_off_t *last_block,
+					      unsigned int nblocks,
+					      bool ra)
 {
 	struct inode *inode = mapping->host;
 	erofs_off_t current_block = (erofs_off_t)page->index;
@@ -233,8 +233,8 @@ static inline struct bio *erofs_read_raw_page(
 
 	/* note that for readpage case, bio also equals to NULL */
 	if (bio != NULL &&
-		/* not continuous */
-		*last_block + 1 != current_block) {
+	    /* not continuous */
+	    *last_block + 1 != current_block) {
 submit_bio_retry:
 		__submit_bio(bio, REQ_OP_READ, 0);
 		bio = NULL;
@@ -307,7 +307,7 @@ static inline struct bio *erofs_read_raw_page(
 			nblocks = BIO_MAX_PAGES;
 
 		bio = erofs_grab_bio(inode->i_sb,
-			blknr, nblocks, read_endio, false);
+				     blknr, nblocks, read_endio, false);
 
 		if (IS_ERR(bio)) {
 			err = PTR_ERR(bio);
@@ -361,7 +361,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 	trace_erofs_readpage(page, true);
 
 	bio = erofs_read_raw_page(NULL, page->mapping,
-		page, &last_block, 1, false);
+				  page, &last_block, 1, false);
 
 	if (IS_ERR(bio))
 		return PTR_ERR(bio);
@@ -371,8 +371,9 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
 }
 
 static int erofs_raw_access_readpages(struct file *filp,
-	struct address_space *mapping,
-	struct list_head *pages, unsigned int nr_pages)
+				      struct address_space *mapping,
+				      struct list_head *pages,
+				      unsigned int nr_pages)
 {
 	erofs_off_t last_block;
 	struct bio *bio = NULL;
@@ -389,13 +390,13 @@ static int erofs_raw_access_readpages(struct file *filp,
 
 		if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) {
 			bio = erofs_read_raw_page(bio, mapping, page,
-				&last_block, nr_pages, true);
+						  &last_block, nr_pages, true);
 
 			/* all the page errors are ignored when readahead */
 			if (IS_ERR(bio)) {
 				pr_err("%s, readahead error at page %lu of nid %llu\n",
-					__func__, page->index,
-					EROFS_V(mapping->host)->nid);
+				       __func__, page->index,
+				       EROFS_V(mapping->host)->nid);
 
 				bio = NULL;
 			}
-- 
2.19.1

[PATCH v3 2/2] staging: erofs: Modify conditional checks

[Bhagyashri P. Dighole]

Fix coding style alignment issues detected by checkpatch.pl
Use ! for NULL test rather than explicitly comparing to NULL.

Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
---
Changes in v3:
- Split patch in two patches. Each patch have similar       
  kind of changes. One patch for all alignment fixes and 
    second patch for all conditional check fixes.

Changes in v2:
- Fix for one alignment issue and one conditional check issue.

Changes in v1:
- Fix for all issues in data.c

 drivers/staging/erofs/data.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
index b66191ad44aa..5a55f0bfdfbb 100644
--- a/drivers/staging/erofs/data.c
+++ b/drivers/staging/erofs/data.c
@@ -53,7 +53,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
 
 repeat:
 	page = find_or_create_page(mapping, blkaddr, gfp);
-	if (unlikely(page == NULL)) {
+	if (unlikely(!page)) {
 		DBG_BUGON(nofail);
 		return ERR_PTR(-ENOMEM);
 	}
@@ -177,7 +177,7 @@ int erofs_map_blocks_iter(struct inode *inode,
 {
 	/* by default, reading raw data never use erofs_map_blocks_iter */
 	if (unlikely(!is_inode_layout_compression(inode))) {
-		if (*mpage_ret != NULL)
+		if (*mpage_ret)
 			put_page(*mpage_ret);
 		*mpage_ret = NULL;
 
@@ -200,7 +200,7 @@ int erofs_map_blocks(struct inode *inode,
 		int err;
 
 		err = erofs_map_blocks_iter(inode, map, &mpage, flags);
-		if (mpage != NULL)
+		if (mpage)
 			put_page(mpage);
 		return err;
 	}
@@ -232,7 +232,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	}
 
 	/* note that for readpage case, bio also equals to NULL */
-	if (bio != NULL &&
+	if (bio &&
 	    /* not continuous */
 	    *last_block + 1 != current_block) {
 submit_bio_retry:
@@ -240,7 +240,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 		bio = NULL;
 	}
 
-	if (bio == NULL) {
+	if (!bio) {
 		struct erofs_map_blocks map = {
 			.m_la = blknr_to_addr(current_block),
 		};
@@ -342,7 +342,7 @@ static inline struct bio *erofs_read_raw_page(struct bio *bio,
 	unlock_page(page);
 
 	/* if updated manually, continuous pages has a gap */
-	if (bio != NULL)
+	if (bio)
 submit_bio_out:
 		__submit_bio(bio, REQ_OP_READ, 0);
 
@@ -408,7 +408,7 @@ static int erofs_raw_access_readpages(struct file *filp,
 	DBG_BUGON(!list_empty(pages));
 
 	/* the rare case (end in gaps) */
-	if (unlikely(bio != NULL))
+	if (unlikely(bio))
 		__submit_bio(bio, REQ_OP_READ, 0);
 	return 0;
 }
-- 
2.19.1

[PATCH v3 1/2] staging: erofs: Modify coding style alignments

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/5 22:49, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Matched alignments with parenthesis.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>

This patch looks good to me. I think you could add, could be better to hear others idea...
Reviewed-by: Gao Xiang <gaoxiang25 at huawei.com>

Thanks,
Gao Xiang

> ---
> Changes in v3:
> - Split patch in two patches. Each patch have similar 
>   kind of changes. One patch for all alignment fixes and 
>   second patch for all conditional check fixes.
> 
> Changes in v2:
> - Fix for one alignment issue and one conditional check issue.
> 
> Changes in v1:
> - Fix for all issues in data.c
> 
>  drivers/staging/erofs/data.c | 51 ++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index 6384f73e5418..b66191ad44aa 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -40,7 +40,7 @@ static inline void read_endio(struct bio *bio)
>  
>  /* prio -- true is used for dir */
>  struct page *__erofs_get_meta_page(struct super_block *sb,
> -	erofs_blk_t blkaddr, bool prio, bool nofail)
> +				   erofs_blk_t blkaddr, bool prio, bool nofail)
>  {
>  	struct inode *const bd_inode = sb->s_bdev->bd_inode;
>  	struct address_space *const mapping = bd_inode->i_mapping;
> @@ -76,7 +76,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
>  		}
>  
>  		__submit_bio(bio, REQ_OP_READ,
> -			REQ_META | (prio ? REQ_PRIO : 0));
> +			     REQ_META | (prio ? REQ_PRIO : 0));
>  
>  		lock_page(page);
>  
> @@ -107,8 +107,8 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
>  }
>  
>  static int erofs_map_blocks_flatmode(struct inode *inode,
> -	struct erofs_map_blocks *map,
> -	int flags)
> +				     struct erofs_map_blocks *map,
> +				     int flags)
>  {
>  	int err = 0;
>  	erofs_blk_t nblocks, lastblk;
> @@ -151,7 +151,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  		map->m_flags |= EROFS_MAP_META;
>  	} else {
>  		errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
> -			vi->nid, inode->i_size, map->m_la);
> +		      vi->nid, inode->i_size, map->m_la);
>  		DBG_BUGON(1);
>  		err = -EIO;
>  		goto err_out;
> @@ -167,12 +167,13 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  
>  #ifdef CONFIG_EROFS_FS_ZIP
>  extern int z_erofs_map_blocks_iter(struct inode *,
> -	struct erofs_map_blocks *, struct page **, int);
> +				   struct erofs_map_blocks *,
> +				   struct page **, int);
>  #endif
>  
>  int erofs_map_blocks_iter(struct inode *inode,
> -	struct erofs_map_blocks *map,
> -	struct page **mpage_ret, int flags)
> +			  struct erofs_map_blocks *map,
> +			  struct page **mpage_ret, int flags)
>  {
>  	/* by default, reading raw data never use erofs_map_blocks_iter */
>  	if (unlikely(!is_inode_layout_compression(inode))) {
> @@ -192,7 +193,7 @@ int erofs_map_blocks_iter(struct inode *inode,
>  }
>  
>  int erofs_map_blocks(struct inode *inode,
> -	struct erofs_map_blocks *map, int flags)
> +		     struct erofs_map_blocks *map, int flags)
>  {
>  	if (unlikely(is_inode_layout_compression(inode))) {
>  		struct page *mpage = NULL;
> @@ -206,13 +207,12 @@ int erofs_map_blocks(struct inode *inode,
>  	return erofs_map_blocks_flatmode(inode, map, flags);
>  }
>  
> -static inline struct bio *erofs_read_raw_page(
> -	struct bio *bio,
> -	struct address_space *mapping,
> -	struct page *page,
> -	erofs_off_t *last_block,
> -	unsigned int nblocks,
> -	bool ra)
> +static inline struct bio *erofs_read_raw_page(struct bio *bio,
> +					      struct address_space *mapping,
> +					      struct page *page,
> +					      erofs_off_t *last_block,
> +					      unsigned int nblocks,
> +					      bool ra)
>  {
>  	struct inode *inode = mapping->host;
>  	erofs_off_t current_block = (erofs_off_t)page->index;
> @@ -233,8 +233,8 @@ static inline struct bio *erofs_read_raw_page(
>  
>  	/* note that for readpage case, bio also equals to NULL */
>  	if (bio != NULL &&
> -		/* not continuous */
> -		*last_block + 1 != current_block) {
> +	    /* not continuous */
> +	    *last_block + 1 != current_block) {
>  submit_bio_retry:
>  		__submit_bio(bio, REQ_OP_READ, 0);
>  		bio = NULL;
> @@ -307,7 +307,7 @@ static inline struct bio *erofs_read_raw_page(
>  			nblocks = BIO_MAX_PAGES;
>  
>  		bio = erofs_grab_bio(inode->i_sb,
> -			blknr, nblocks, read_endio, false);
> +				     blknr, nblocks, read_endio, false);
>  
>  		if (IS_ERR(bio)) {
>  			err = PTR_ERR(bio);
> @@ -361,7 +361,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
>  	trace_erofs_readpage(page, true);
>  
>  	bio = erofs_read_raw_page(NULL, page->mapping,
> -		page, &last_block, 1, false);
> +				  page, &last_block, 1, false);
>  
>  	if (IS_ERR(bio))
>  		return PTR_ERR(bio);
> @@ -371,8 +371,9 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
>  }
>  
>  static int erofs_raw_access_readpages(struct file *filp,
> -	struct address_space *mapping,
> -	struct list_head *pages, unsigned int nr_pages)
> +				      struct address_space *mapping,
> +				      struct list_head *pages,
> +				      unsigned int nr_pages)
>  {
>  	erofs_off_t last_block;
>  	struct bio *bio = NULL;
> @@ -389,13 +390,13 @@ static int erofs_raw_access_readpages(struct file *filp,
>  
>  		if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) {
>  			bio = erofs_read_raw_page(bio, mapping, page,
> -				&last_block, nr_pages, true);
> +						  &last_block, nr_pages, true);
>  
>  			/* all the page errors are ignored when readahead */
>  			if (IS_ERR(bio)) {
>  				pr_err("%s, readahead error at page %lu of nid %llu\n",
> -					__func__, page->index,
> -					EROFS_V(mapping->host)->nid);
> +				       __func__, page->index,
> +				       EROFS_V(mapping->host)->nid);
>  
>  				bio = NULL;
>  			}
> 

Re: [PATCH v3 1/2] staging: erofs: Modify coding style alignments

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/5 22:49, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Matched alignments with parenthesis.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>

This patch looks good to me. I think you could add, could be better to hear others idea...
Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> ---
> Changes in v3:
> - Split patch in two patches. Each patch have similar 
>   kind of changes. One patch for all alignment fixes and 
>   second patch for all conditional check fixes.
> 
> Changes in v2:
> - Fix for one alignment issue and one conditional check issue.
> 
> Changes in v1:
> - Fix for all issues in data.c
> 
>  drivers/staging/erofs/data.c | 51 ++++++++++++++++++------------------
>  1 file changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/staging/erofs/data.c b/drivers/staging/erofs/data.c
> index 6384f73e5418..b66191ad44aa 100644
> --- a/drivers/staging/erofs/data.c
> +++ b/drivers/staging/erofs/data.c
> @@ -40,7 +40,7 @@ static inline void read_endio(struct bio *bio)
>  
>  /* prio -- true is used for dir */
>  struct page *__erofs_get_meta_page(struct super_block *sb,
> -	erofs_blk_t blkaddr, bool prio, bool nofail)
> +				   erofs_blk_t blkaddr, bool prio, bool nofail)
>  {
>  	struct inode *const bd_inode = sb->s_bdev->bd_inode;
>  	struct address_space *const mapping = bd_inode->i_mapping;
> @@ -76,7 +76,7 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
>  		}
>  
>  		__submit_bio(bio, REQ_OP_READ,
> -			REQ_META | (prio ? REQ_PRIO : 0));
> +			     REQ_META | (prio ? REQ_PRIO : 0));
>  
>  		lock_page(page);
>  
> @@ -107,8 +107,8 @@ struct page *__erofs_get_meta_page(struct super_block *sb,
>  }
>  
>  static int erofs_map_blocks_flatmode(struct inode *inode,
> -	struct erofs_map_blocks *map,
> -	int flags)
> +				     struct erofs_map_blocks *map,
> +				     int flags)
>  {
>  	int err = 0;
>  	erofs_blk_t nblocks, lastblk;
> @@ -151,7 +151,7 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  		map->m_flags |= EROFS_MAP_META;
>  	} else {
>  		errln("internal error @ nid: %llu (size %llu), m_la 0x%llx",
> -			vi->nid, inode->i_size, map->m_la);
> +		      vi->nid, inode->i_size, map->m_la);
>  		DBG_BUGON(1);
>  		err = -EIO;
>  		goto err_out;
> @@ -167,12 +167,13 @@ static int erofs_map_blocks_flatmode(struct inode *inode,
>  
>  #ifdef CONFIG_EROFS_FS_ZIP
>  extern int z_erofs_map_blocks_iter(struct inode *,
> -	struct erofs_map_blocks *, struct page **, int);
> +				   struct erofs_map_blocks *,
> +				   struct page **, int);
>  #endif
>  
>  int erofs_map_blocks_iter(struct inode *inode,
> -	struct erofs_map_blocks *map,
> -	struct page **mpage_ret, int flags)
> +			  struct erofs_map_blocks *map,
> +			  struct page **mpage_ret, int flags)
>  {
>  	/* by default, reading raw data never use erofs_map_blocks_iter */
>  	if (unlikely(!is_inode_layout_compression(inode))) {
> @@ -192,7 +193,7 @@ int erofs_map_blocks_iter(struct inode *inode,
>  }
>  
>  int erofs_map_blocks(struct inode *inode,
> -	struct erofs_map_blocks *map, int flags)
> +		     struct erofs_map_blocks *map, int flags)
>  {
>  	if (unlikely(is_inode_layout_compression(inode))) {
>  		struct page *mpage = NULL;
> @@ -206,13 +207,12 @@ int erofs_map_blocks(struct inode *inode,
>  	return erofs_map_blocks_flatmode(inode, map, flags);
>  }
>  
> -static inline struct bio *erofs_read_raw_page(
> -	struct bio *bio,
> -	struct address_space *mapping,
> -	struct page *page,
> -	erofs_off_t *last_block,
> -	unsigned int nblocks,
> -	bool ra)
> +static inline struct bio *erofs_read_raw_page(struct bio *bio,
> +					      struct address_space *mapping,
> +					      struct page *page,
> +					      erofs_off_t *last_block,
> +					      unsigned int nblocks,
> +					      bool ra)
>  {
>  	struct inode *inode = mapping->host;
>  	erofs_off_t current_block = (erofs_off_t)page->index;
> @@ -233,8 +233,8 @@ static inline struct bio *erofs_read_raw_page(
>  
>  	/* note that for readpage case, bio also equals to NULL */
>  	if (bio != NULL &&
> -		/* not continuous */
> -		*last_block + 1 != current_block) {
> +	    /* not continuous */
> +	    *last_block + 1 != current_block) {
>  submit_bio_retry:
>  		__submit_bio(bio, REQ_OP_READ, 0);
>  		bio = NULL;
> @@ -307,7 +307,7 @@ static inline struct bio *erofs_read_raw_page(
>  			nblocks = BIO_MAX_PAGES;
>  
>  		bio = erofs_grab_bio(inode->i_sb,
> -			blknr, nblocks, read_endio, false);
> +				     blknr, nblocks, read_endio, false);
>  
>  		if (IS_ERR(bio)) {
>  			err = PTR_ERR(bio);
> @@ -361,7 +361,7 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
>  	trace_erofs_readpage(page, true);
>  
>  	bio = erofs_read_raw_page(NULL, page->mapping,
> -		page, &last_block, 1, false);
> +				  page, &last_block, 1, false);
>  
>  	if (IS_ERR(bio))
>  		return PTR_ERR(bio);
> @@ -371,8 +371,9 @@ static int erofs_raw_access_readpage(struct file *file, struct page *page)
>  }
>  
>  static int erofs_raw_access_readpages(struct file *filp,
> -	struct address_space *mapping,
> -	struct list_head *pages, unsigned int nr_pages)
> +				      struct address_space *mapping,
> +				      struct list_head *pages,
> +				      unsigned int nr_pages)
>  {
>  	erofs_off_t last_block;
>  	struct bio *bio = NULL;
> @@ -389,13 +390,13 @@ static int erofs_raw_access_readpages(struct file *filp,
>  
>  		if (!add_to_page_cache_lru(page, mapping, page->index, gfp)) {
>  			bio = erofs_read_raw_page(bio, mapping, page,
> -				&last_block, nr_pages, true);
> +						  &last_block, nr_pages, true);
>  
>  			/* all the page errors are ignored when readahead */
>  			if (IS_ERR(bio)) {
>  				pr_err("%s, readahead error at page %lu of nid %llu\n",
> -					__func__, page->index,
> -					EROFS_V(mapping->host)->nid);
> +				       __func__, page->index,
> +				       EROFS_V(mapping->host)->nid);
>  
>  				bio = NULL;
>  			}
> 

[PATCH v3 2/2] staging: erofs: Modify conditional checks

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Use ! for NULL test rather than explicitly comparing to NULL.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
> ---
> Changes in v3:
> - Split patch in two patches. Each patch have similar       
>   kind of changes. One patch for all alignment fixes and 
>     second patch for all conditional check fixes.
> 
> Changes in v2:
> - Fix for one alignment issue and one conditional check issue.
> 
> Changes in v1:
> - Fix for all issues in data.c

As the Greg said before, there are too many patchset v3... :(
Apart from its content, could you please resend the whole patchset
by using a proper version number...

It is really confusing for me as well...

Thanks,
Gao Xiang

Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> Fix coding style alignment issues detected by checkpatch.pl
> Use ! for NULL test rather than explicitly comparing to NULL.
> 
> Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
> ---
> Changes in v3:
> - Split patch in two patches. Each patch have similar       
>   kind of changes. One patch for all alignment fixes and 
>     second patch for all conditional check fixes.
> 
> Changes in v2:
> - Fix for one alignment issue and one conditional check issue.
> 
> Changes in v1:
> - Fix for all issues in data.c

As the Greg said before, there are too many patchset v3... :(
Apart from its content, could you please resend the whole patchset
by using a proper version number...

It is really confusing for me as well...

Thanks,
Gao Xiang

[Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Julia Lawall]

On Mon, 5 Nov 2018, Gao Xiang wrote:

> Hi Bhagyashri,
>
> On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> > Fix coding style alignment issues detected by checkpatch.pl
> > Use ! for NULL test rather than explicitly comparing to NULL.
> >
> > Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
> > ---
> > Changes in v3:
> > - Split patch in two patches. Each patch have similar
> >   kind of changes. One patch for all alignment fixes and
> >     second patch for all conditional check fixes.
> >
> > Changes in v2:
> > - Fix for one alignment issue and one conditional check issue.
> >
> > Changes in v1:
> > - Fix for all issues in data.c
>
> As the Greg said before, there are too many patchset v3... :(
> Apart from its content, could you please resend the whole patchset
> by using a proper version number...
>
> It is really confusing for me as well...

Bhagyashri,

When you resend, you should include the Reviewed-by that you received.

julia


>
> Thanks,
> Gao Xiang
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe at googlegroups.com.
> To post to this group, send email to outreachy-kernel at googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/3ac63350-6bfc-0dba-2649-ad7f91dd94c9%40huawei.com.
> For more options, visit https://groups.google.com/d/optout.
>

Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Julia Lawall]

On Mon, 5 Nov 2018, Gao Xiang wrote:

> Hi Bhagyashri,
>
> On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> > Fix coding style alignment issues detected by checkpatch.pl
> > Use ! for NULL test rather than explicitly comparing to NULL.
> >
> > Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
> > ---
> > Changes in v3:
> > - Split patch in two patches. Each patch have similar
> >   kind of changes. One patch for all alignment fixes and
> >     second patch for all conditional check fixes.
> >
> > Changes in v2:
> > - Fix for one alignment issue and one conditional check issue.
> >
> > Changes in v1:
> > - Fix for all issues in data.c
>
> As the Greg said before, there are too many patchset v3... :(
> Apart from its content, could you please resend the whole patchset
> by using a proper version number...
>
> It is really confusing for me as well...

Bhagyashri,

When you resend, you should include the Reviewed-by that you received.

julia


>
> Thanks,
> Gao Xiang
>
> --
> You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> To post to this group, send email to outreachy-kernel@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/3ac63350-6bfc-0dba-2649-ad7f91dd94c9%40huawei.com.
> For more options, visit https://groups.google.com/d/optout.
>

[Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Bhagyashri Dighole]

Hi Gao,

In PATCH version v1 which is first stage of contribution, I modified data.c
for all alignment changes and all conditional checks. As per discussion on
email for this patch, It was concluded that I need to re-submit patch for
single change at a time.

So, I created PATCH v2. In this patch, I modified data.c only for single
conditional check and single alignment change and committed accordingly. As
per Gao?s suggestion to do all alignment change in a first patch and all
conditional changes in another patch, I modified and created PATCH v3.

PATCH v3 is divided into two patches.
1) PATCH v3 1/2 has all alignment changes.
2) PATCH v3 2/2 has all conditional changes.

Also, I mentioned what changes each version of patch represents under ??-?
symbol.

I am not able to understand, where I am making mistake? Can you please help
me?


@Julia Lawall <julia.lawall at lip6.fr>  How can I include Reviewed-by in
patch submission? we are allow to do that? If yes, then from where I can
get that information?

Regards

On Mon, Nov 5, 2018@9:21 PM Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Mon, 5 Nov 2018, Gao Xiang wrote:
>
> > Hi Bhagyashri,
> >
> > On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> > > Fix coding style alignment issues detected by checkpatch.pl
> > > Use ! for NULL test rather than explicitly comparing to NULL.
> > >
> > > Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri at gmail.com>
> > > ---
> > > Changes in v3:
> > > - Split patch in two patches. Each patch have similar
> > >   kind of changes. One patch for all alignment fixes and
> > >     second patch for all conditional check fixes.
> > >
> > > Changes in v2:
> > > - Fix for one alignment issue and one conditional check issue.
> > >
> > > Changes in v1:
> > > - Fix for all issues in data.c
> >
> > As the Greg said before, there are too many patchset v3... :(
> > Apart from its content, could you please resend the whole patchset
> > by using a proper version number...
> >
> > It is really confusing for me as well...
>
> Bhagyashri,
>
> When you resend, you should include the Reviewed-by that you received.
>
> julia
>
>
> >
> > Thanks,
> > Gao Xiang
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe at googlegroups.com.
> > To post to this group, send email to outreachy-kernel at googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/3ac63350-6bfc-0dba-2649-ad7f91dd94c9%40huawei.com
> .
> > For more options, visit https://groups.google.com/d/optout.
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.ozlabs.org/pipermail/linux-erofs/attachments/20181105/37adce56/attachment.html>

Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Bhagyashri Dighole]

[-- Attachment #1: Type: text/plain, Size: 2809 bytes --]

Hi Gao,

In PATCH version v1 which is first stage of contribution, I modified data.c
for all alignment changes and all conditional checks. As per discussion on
email for this patch, It was concluded that I need to re-submit patch for
single change at a time.

So, I created PATCH v2. In this patch, I modified data.c only for single
conditional check and single alignment change and committed accordingly. As
per Gao’s suggestion to do all alignment change in a first patch and all
conditional changes in another patch, I modified and created PATCH v3.

PATCH v3 is divided into two patches.
1) PATCH v3 1/2 has all alignment changes.
2) PATCH v3 2/2 has all conditional changes.

Also, I mentioned what changes each version of patch represents under ‘—-‘
symbol.

I am not able to understand, where I am making mistake? Can you please help
me?


@Julia Lawall <julia.lawall@lip6.fr>  How can I include Reviewed-by in
patch submission? we are allow to do that? If yes, then from where I can
get that information?

Regards

On Mon, Nov 5, 2018 at 9:21 PM Julia Lawall <julia.lawall@lip6.fr> wrote:

>
>
> On Mon, 5 Nov 2018, Gao Xiang wrote:
>
> > Hi Bhagyashri,
> >
> > On 2018/11/5 22:50, Bhagyashri P. Dighole wrote:
> > > Fix coding style alignment issues detected by checkpatch.pl
> > > Use ! for NULL test rather than explicitly comparing to NULL.
> > >
> > > Signed-off-by: Bhagyashri P. Dighole <digholebhagyashri@gmail.com>
> > > ---
> > > Changes in v3:
> > > - Split patch in two patches. Each patch have similar
> > >   kind of changes. One patch for all alignment fixes and
> > >     second patch for all conditional check fixes.
> > >
> > > Changes in v2:
> > > - Fix for one alignment issue and one conditional check issue.
> > >
> > > Changes in v1:
> > > - Fix for all issues in data.c
> >
> > As the Greg said before, there are too many patchset v3... :(
> > Apart from its content, could you please resend the whole patchset
> > by using a proper version number...
> >
> > It is really confusing for me as well...
>
> Bhagyashri,
>
> When you resend, you should include the Reviewed-by that you received.
>
> julia
>
>
> >
> > Thanks,
> > Gao Xiang
> >
> > --
> > You received this message because you are subscribed to the Google
> Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it, send
> an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To post to this group, send email to outreachy-kernel@googlegroups.com.
> > To view this discussion on the web visit
> https://groups.google.com/d/msgid/outreachy-kernel/3ac63350-6bfc-0dba-2649-ad7f91dd94c9%40huawei.com
> .
> > For more options, visit https://groups.google.com/d/optout.
> >
>

[-- Attachment #2: Type: text/html, Size: 5647 bytes --]

[Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/6 2:07, Bhagyashri Dighole wrote:
> Hi Gao,
> 
> In PATCH version v1 which is first stage of contribution, I modified data.c for all alignment changes and all conditional checks.?As per discussion on email for this patch, It was concluded that I need to re-submit patch for single change at a time.
> 
> So, I created PATCH v2. In this patch, I modified data.c only for single conditional check and single alignment change and committed accordingly. As per Gao?s suggestion to do all alignment change in a first patch and all conditional changes in another patch, I modified and created PATCH v3.
> 
> PATCH v3 is divided into two patches.
> 1) PATCH v3 1/2 has all alignment changes.
> 2) PATCH v3 2/2 has all conditional changes.

That is correct. However, you send two patchset of v3, see:

v3. the first time
1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000796.html
2) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000797.html
2.1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000798.html
v3. the second time
1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000800.html
2) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000801.html

It is hard for me to know which one is the latest version of your patchset.
If you send a newer version, you should use v4 instead of v3 (even if you
decide to add a changelog), or

If you resend the patchset (same as the previous one, just to remind us to review),
  you should use [PATCH v3 RESEND] for this case.

> 
> Also, I mentioned what changes each version of patch represents under ??-? symbol.

Yes, but I think you should use v4 rather than v3 to add a changelog as Greg mentioned.

> 
> I am not able to understand, where I am making mistake??Can you please help me?

That is my personal suggestion, maybe of some help for you...

Thanks,
Gao Xiang

Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Gao Xiang]

Hi Bhagyashri,

On 2018/11/6 2:07, Bhagyashri Dighole wrote:
> Hi Gao,
> 
> In PATCH version v1 which is first stage of contribution, I modified data.c for all alignment changes and all conditional checks. As per discussion on email for this patch, It was concluded that I need to re-submit patch for single change at a time.
> 
> So, I created PATCH v2. In this patch, I modified data.c only for single conditional check and single alignment change and committed accordingly. As per Gao’s suggestion to do all alignment change in a first patch and all conditional changes in another patch, I modified and created PATCH v3.
> 
> PATCH v3 is divided into two patches.
> 1) PATCH v3 1/2 has all alignment changes.
> 2) PATCH v3 2/2 has all conditional changes.

That is correct. However, you send two patchset of v3, see:

v3. the first time
1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000796.html
2) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000797.html
2.1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000798.html
v3. the second time
1) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000800.html
2) https://lists.ozlabs.org/pipermail/linux-erofs/2018-November/000801.html

It is hard for me to know which one is the latest version of your patchset.
If you send a newer version, you should use v4 instead of v3 (even if you
decide to add a changelog), or

If you resend the patchset (same as the previous one, just to remind us to review),
  you should use [PATCH v3 RESEND] for this case.

> 
> Also, I mentioned what changes each version of patch represents under ‘—-‘ symbol.

Yes, but I think you should use v4 rather than v3 to add a changelog as Greg mentioned.

> 
> I am not able to understand, where I am making mistake? Can you please help me?

That is my personal suggestion, maybe of some help for you...

Thanks,
Gao Xiang

[Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Himanshu Jha]

On Mon, Nov 05, 2018@11:37:53PM +0530, Bhagyashri Dighole wrote:
> Hi Gao,
> 
> In PATCH version v1 which is first stage of contribution, I modified data.c
> for all alignment changes and all conditional checks. As per discussion on
> email for this patch, It was concluded that I need to re-submit patch for
> single change at a time.
> 
> So, I created PATCH v2. In this patch, I modified data.c only for single
> conditional check and single alignment change and committed accordingly. As
> per Gao?s suggestion to do all alignment change in a first patch and all
> conditional changes in another patch, I modified and created PATCH v3.
> 
> PATCH v3 is divided into two patches.
> 1) PATCH v3 1/2 has all alignment changes.
> 2) PATCH v3 2/2 has all conditional changes.

I saw two variants for 2)

check your `sent` box once and you will understand what the problem is.

> Also, I mentioned what changes each version of patch represents under ??-?
> symbol.

But I didn't see for one of the variants for 2)

> I am not able to understand, where I am making mistake? Can you please help
> me?

When someone says to resend a patch use subject as [RESEND PATCH ... ..]

See how development goes out here https://lkml.org/

> @Julia Lawall <julia.lawall at lip6.fr>  How can I include Reviewed-by in
> patch submission? we are allow to do that? If yes, then from where I can
> get that information?

Julia misunderstood I guess due identical patches and now when you send
a new version add Gao's `Reviewed-by` tag with proper changelog and
versioning.

Lastly, Please don't top-post and don't bomb the outreachy mailing list.


@Ioannis Valasakis,

If you're following the list, then please understand what reviews you
get carefully. You made the exactly same mistake in new series as Julia
pointed and was earlier pointed by me.

You're lucky to have Julia, Greg, Sasha *patiently* replying and perhaps you
know about their mainline work. So, please rest their time and effort.


Otherwise, in future very likely :P 
=> => https://lore.kernel.org/lkml/20170829142011.76w67w64a7lheef4 at piout.net/


Thanks!
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology

Re: [Outreachy kernel] Re: [PATCH v3 2/2] staging: erofs: Modify conditional checks

[Himanshu Jha]

On Mon, Nov 05, 2018 at 11:37:53PM +0530, Bhagyashri Dighole wrote:
> Hi Gao,
> 
> In PATCH version v1 which is first stage of contribution, I modified data.c
> for all alignment changes and all conditional checks. As per discussion on
> email for this patch, It was concluded that I need to re-submit patch for
> single change at a time.
> 
> So, I created PATCH v2. In this patch, I modified data.c only for single
> conditional check and single alignment change and committed accordingly. As
> per Gao’s suggestion to do all alignment change in a first patch and all
> conditional changes in another patch, I modified and created PATCH v3.
> 
> PATCH v3 is divided into two patches.
> 1) PATCH v3 1/2 has all alignment changes.
> 2) PATCH v3 2/2 has all conditional changes.

I saw two variants for 2)

check your `sent` box once and you will understand what the problem is.

> Also, I mentioned what changes each version of patch represents under ‘—-‘
> symbol.

But I didn't see for one of the variants for 2)

> I am not able to understand, where I am making mistake? Can you please help
> me?

When someone says to resend a patch use subject as [RESEND PATCH ... ..]

See how development goes out here https://lkml.org/

> @Julia Lawall <julia.lawall@lip6.fr>  How can I include Reviewed-by in
> patch submission? we are allow to do that? If yes, then from where I can
> get that information?

Julia misunderstood I guess due identical patches and now when you send
a new version add Gao's `Reviewed-by` tag with proper changelog and
versioning.

Lastly, Please don't top-post and don't bomb the outreachy mailing list.


@Ioannis Valasakis,

If you're following the list, then please understand what reviews you
get carefully. You made the exactly same mistake in new series as Julia
pointed and was earlier pointed by me.

You're lucky to have Julia, Greg, Sasha *patiently* replying and perhaps you
know about their mainline work. So, please rest their time and effort.


Otherwise, in future very likely :P 
=> => https://lore.kernel.org/lkml/20170829142011.76w67w64a7lheef4@piout.net/


Thanks!
-- 
Himanshu Jha
Undergraduate Student
Department of Electronics & Communication
Guru Tegh Bahadur Institute of Technology