[PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c

[PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch warnings by
adding "int" to the unsigned type.

The patch fixes 11 warnings of the type:
"WARNING: Prefer 'unsigned int' to bare use of 'unsigned'"

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index f5b665f..f9e6b78 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -23,14 +23,14 @@ static struct {
 } erofs_pcpubuf[NR_CPUS];
 
 int z_erofs_vle_plain_copy(struct page **compressed_pages,
-			   unsigned clusterpages,
+			   unsigned int clusterpages,
 			   struct page **pages,
-			   unsigned nr_pages,
+			   unsigned int nr_pages,
 			   unsigned short pageofs)
 {
-	unsigned i, j;
+	unsigned int i, j;
 	void *src = NULL;
-	const unsigned righthalf = PAGE_SIZE - pageofs;
+	const unsigned int righthalf = PAGE_SIZE - pageofs;
 	char *percpu_data;
 	bool mirrored[Z_EROFS_CLUSTER_MAX_PAGES] = { 0 };
 
@@ -102,14 +102,14 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 extern int z_erofs_unzip_lz4(void *in, void *out, size_t inlen, size_t outlen);
 
 int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
-				  unsigned clusterpages,
+				  unsigned int clusterpages,
 				  struct page **pages,
-				  unsigned outlen,
+				  unsigned int outlen,
 				  unsigned short pageofs,
 				  void (*endio)(struct page *))
 {
 	void *vin, *vout;
-	unsigned nr_pages, i, j;
+	unsigned int nr_pages, i, j;
 	int ret;
 
 	if (outlen + pageofs > EROFS_PERCPU_NR_PAGES * PAGE_SIZE)
@@ -134,7 +134,7 @@ int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
 	}
 
 	for (i = 0; i < nr_pages; ++i) {
-		j = min((unsigned)PAGE_SIZE - pageofs, outlen);
+		j = min((unsigned int)PAGE_SIZE - pageofs, outlen);
 
 		if (pages[i] != NULL) {
 			if (ret < 0)
@@ -164,14 +164,14 @@ int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
 }
 
 int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
-			   unsigned clusterpages,
+			   unsigned int clusterpages,
 			   void *vout,
-			   unsigned llen,
+			   unsigned int llen,
 			   unsigned short pageofs,
 			   bool overlapped)
 {
 	void *vin;
-	unsigned i;
+	unsigned int i;
 	int ret;
 
 	if (overlapped) {
@@ -206,4 +206,3 @@ int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
 
 	return ret;
 }
-
-- 
2.7.4

[PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch checks by
to NULL comparison.

The patch fixes 5 checks of type:
"Comparison to NULL could be written".

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index f9e6b78..54da72a 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -42,8 +42,8 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 		struct page *page = pages[i];
 		void *dst;
 
-		if (page == NULL) {
-			if (src != NULL) {
+		if (!page) {
+			if (src) {
 				if (!mirrored[j])
 					kunmap_atomic(src);
 				src = NULL;
@@ -64,7 +64,7 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 		}
 
 		if (i) {
-			if (src == NULL)
+			if (!src)
 				src = mirrored[i-1] ?
 					percpu_data + (i-1) * PAGE_SIZE :
 					kmap_atomic(compressed_pages[i-1]);
@@ -92,7 +92,7 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 		kunmap_atomic(dst);
 	}
 
-	if (src != NULL && !mirrored[j])
+	if (src && !mirrored[j])
 		kunmap_atomic(src);
 
 	preempt_enable();
@@ -136,7 +136,7 @@ int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
 	for (i = 0; i < nr_pages; ++i) {
 		j = min((unsigned int)PAGE_SIZE - pageofs, outlen);
 
-		if (pages[i] != NULL) {
+		if (pages[i]) {
 			if (ret < 0)
 				SetPageError(pages[i]);
 			else if (clusterpages == 1 && pages[i] == compressed_pages[0])
-- 
2.7.4

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Gao Xiang]

Hi Pavel,

On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch checks by
> to NULL comparison.
> 
> The patch fixes 5 checks of type:
> "Comparison to NULL could be written".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Sorry about the late reply. I am on vacation now.

Personally, I use "== NULL" or "!= NULL" on purpose, since it is more 
emphasized than the checkpatch.pl suggested way, and I tend to use the nullptr explicitly.

BTW, It seems other filesystems still use "== NULL" or "!= NULL" explicitly, too, eg:
xfs, ext4, ext2, ocfs2, etc... You could 'grep' in the fs directory... 

Other commits look good for me at glance.

Thanks,
Gao Xiang

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Gao Xiang]

On 2018/8/31 0:09, Gao Xiang via Linux-erofs wrote:
> Hi Pavel,
> 
> On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch checks by
>> to NULL comparison.
>>
>> The patch fixes 5 checks of type:
>> "Comparison to NULL could be written".
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Sorry about the late reply. I am on vacation now.
> 
> Personally, I use "== NULL" or "!= NULL" on purpose, since it is more 
> emphasized than the checkpatch.pl suggested way, and I tend to use the nullptr explicitly.
> 
> BTW, It seems other filesystems still use "== NULL" or "!= NULL" explicitly, too, eg:
> xfs, ext4, ext2, ocfs2, etc... You could 'grep' in the fs directory... 
> 
> Other commits look good for me at glance.
> 

p.s. I personally tend to drop this patch. :(

Here are bunch of 'NULL comparison' usages in xfs, eg.

...
./xfs_qm_syscalls.c:218:        if (ino == NULLFSINO)                                                                                                        ./xfs_qm_syscalls.c:747:                ASSERT(ip->i_udquot == NULL);                                                                                        ./xfs_qm_syscalls.c:748:                ASSERT(ip->i_gdquot == NULL);                                                                                        ./xfs_qm_syscalls.c:749:                ASSERT(ip->i_pdquot == NULL);                                                                                        ./xfs_quota.h:27:       ((XFS_IS_UQUOTA_ON(mp) && (ip)->i_udquot == NULL) || \                                                                               ./xfs_quota.h:28:        (XFS_IS_GQUOTA_ON(mp) && (ip)->i_gdquot == NULL) || \                                                                               ./xfs_quota.h:29:        (XFS_IS_PQUOTA_ON(mp) && (ip)->i_pdquot == NULL))                                                                                   ./xfs_quotaops.c:31:    if (!ip && ino == NULLFSINO)                                                                                                         ./xfs_reflink.c:213:    if (fbno == NULLAGBLOCK) {                                                                                                           ./xfs_reflink.c:652:    if (count == NULLFILEOFF)                                                                                                            ./xfs_reflink.c:1462:                   if (rbno == NULLAGBLOCK)                                                                                             ./xfs_rtalloc.c:836:                    if (bp == NULL) {                                                                                                    ./xfs_rtalloc.c:899:    if (mp->m_rtdev_targp == NULL || mp->m_rbmip == NULL ||                                                                              ./xfs_rtalloc.c:1165:   if (mp->m_rtdev_targp == NULL) {                                                                                                     ./xfs_rtalloc.c:1209:   if (sbp->sb_rbmino == NULLFSINO)                                                                                                     ./xfs_trans.c:185:                      ASSERT(tp->t_ticket == NULL);                                                                                        ./xfs_trans_ail.c:59:       (prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0) &&                                                                ./xfs_trans_ail.c:60:       (next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0))                                                                  ./xfs_trans_ail.c:65:   ASSERT(prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0);                                                                ./xfs_trans_ail.c:66:   ASSERT(next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0);                                                                ./xfs_trans_ail.c:475:          if (lip == NULL)                                                                                                             ./xfs_trans_buf.c:70:   ASSERT(bp->b_transp == NULL);                                                                                                        ./xfs_trans_buf.c:155:  if (bp == NULL) {                                                                                                                    ./xfs_trans_buf.c:187:  if (tp == NULL)                                                                                                                      ./xfs_trans_buf.c:207:  if (bp == NULL)                                                                                                                      ./xfs_trans_buf.c:350:  if (tp == NULL) {                                                                                                                    ./xfs_trans_buf.c:351:          ASSERT(bp->b_transp == NULL);                                                                                                ./xfs_trans_buf.c:495:  ASSERT(bp->b_iodone == NULL ||                                                                                                       ./xfs_trans_dquot.c:103:                        if (oqa[i].qt_dquot == NULL)                                                                                 ./xfs_trans_dquot.c:149:        if (tp->t_dqinfo == NULL)                                                                                                    ./xfs_trans_dquot.c:178:                if (qa[i].qt_dquot == NULL ||                                                                                        ./xfs_trans_dquot.c:205:        if (tp->t_dqinfo == NULL)                                                                                                    ./xfs_trans_dquot.c:213:        if (qtrx->qt_dquot == NULL)                                                                                                  ./xfs_trans_dquot.c:295:        if (q[1].qt_dquot == NULL) {                                                                                                 ./xfs_trans_dquot.c:332:                if (qa[0].qt_dquot == NULL)                                                                                          ./xfs_trans_dquot.c:346:                        if ((dqp = qtrx->qt_dquot) == NULL)                                                                          ./xfs_trans_dquot.c:519:                        if ((dqp = qtrx->qt_dquot) == NULL)                                                                          ./xfs_trans_dquot.c:757:        if (tp && tp->t_dqinfo == NULL)                                                                                              ./xfs_trans_inode.c:36: if (ip->i_itemp == NULL)                                                                                                             
...

And in ext4, eg.
...
./mballoc.c:2892:       if (ext4_free_data_cachep == NULL) {
./mballoc.c:3046:       BUG_ON(lg == NULL);
./mballoc.c:3286:       if (pa == NULL) {
./mballoc.c:3377:       if (cpa == NULL) {
./mballoc.c:3447:       if (lg == NULL)
./mballoc.c:3635:       if (pa == NULL)
./mballoc.c:3727:       BUG_ON(ext4_pspace_cachep == NULL);
./mballoc.c:3729:       if (pa == NULL)
./mballoc.c:3756:       BUG_ON(lg == NULL);
./mballoc.c:4637:       BUG_ON(e4b->bd_bitmap_page == NULL);
./mballoc.c:4638:       BUG_ON(e4b->bd_buddy_page == NULL);
./move_extent.c:34:     if (path[ext_depth(inode)].p_ext == NULL) {
./namei.c:874:  if (frames[0].bh == NULL)
./namei.c:879:          if (frames[i].bh == NULL)
./namei.c:1432:         if ((bh = bh_use[ra_ptr++]) == NULL)
./page-io.c:38: if (io_end_cachep == NULL)
./page-io.c:398:        if (io->io_bio == NULL) {
./readpage.c:242:               if (bio == NULL) {
./resize.c:200: if (flex_gd == NULL)
./resize.c:210: if (flex_gd->groups == NULL)
./resize.c:215: if (flex_gd->bg_flags == NULL)
./resize.c:265: BUG_ON(flex_gd->count == 0 || group_data == NULL);
./resize.c:1345:        BUG_ON(flex_gd->count == 0 || group_data == NULL);
./resize.c:2012:        if (flex_gd == NULL) {
./super.c:365:  return bdi->dev == NULL;
./super.c:596:  if (errno == -EROFS && journal_current_handle() == NULL && sb_rdonly(sb))
./super.c:1086: if (ext4_inode_cachep == NULL)
./super.c:4050: if (sbi->s_group_desc == NULL) {
./super.c:4617: if (bdev == NULL)
./super.c:5288: if (sbi->s_journal == NULL && !(old_sb_flags & SB_RDONLY)) {
./xattr.c:286:  if (name == NULL)
./xattr.c:1905:                 if (s->base == NULL)
./xattr.c:1948:         if (s->base == NULL)
./xattr.c:2665:         if (entry == NULL) {
./xattr.c:2666:                 if (small_entry == NULL)
./xattr.c:2804: if (*ea_inode_array == NULL) {
./xattr.c:2813:         if (*ea_inode_array == NULL)
./xattr.c:2826:         if (new_array == NULL)
./xattr.c:2947: if (ea_inode_array == NULL)
...

Anyway, I'd like to listen the Greg's and Chao's ideas.

Thanks,

> Thanks,
> Gao Xiang
> 

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Chao Yu]

On 2018/8/31 0:32, Gao Xiang via Linux-erofs wrote:
> 
> 
> On 2018/8/31 0:09, Gao Xiang via Linux-erofs wrote:
>> Hi Pavel,
>>
>> On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
>>> This patch does not change the logic, it only
>>> corrects the formatting and checkpatch checks by
>>> to NULL comparison.
>>>
>>> The patch fixes 5 checks of type:
>>> "Comparison to NULL could be written".
>>>
>>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
>>
>> Sorry about the late reply. I am on vacation now.
>>
>> Personally, I use "== NULL" or "!= NULL" on purpose, since it is more 
>> emphasized than the checkpatch.pl suggested way, and I tend to use the nullptr explicitly.
>>
>> BTW, It seems other filesystems still use "== NULL" or "!= NULL" explicitly, too, eg:
>> xfs, ext4, ext2, ocfs2, etc... You could 'grep' in the fs directory... 
>>
>> Other commits look good for me at glance.
>>
> 
> p.s. I personally tend to drop this patch. :(
> 
> Here are bunch of 'NULL comparison' usages in xfs, eg.
> 
> ...
> ./xfs_qm_syscalls.c:218:        if (ino == NULLFSINO)                                                                                                        ./xfs_qm_syscalls.c:747:                ASSERT(ip->i_udquot == NULL);                                                                                        ./xfs_qm_syscalls.c:748:                ASSERT(ip->i_gdquot == NULL);                                                                                        ./xfs_qm_syscalls.c:749:                ASSERT(ip->i_pdquot == NULL);                                                                                        ./xfs_quota.h:27:       ((XFS_IS_UQUOTA_ON(mp) && (ip)->i_udquot == NULL) || \                                                                               ./xfs_quota.h:28:        (XFS_IS_GQUOTA_ON(mp) && (ip)->i_gdquot == NULL) || \                                                                               ./xfs_quota.h:29:        (XFS_IS_PQUOTA_ON(mp) && (ip)->i_pdquot == NULL))                                                                                   ./xfs_quotaops.c:31:    if (!ip && ino == NULLFSINO)                                                                                                         ./xfs_reflink.c:213:    if (fbno == NULLAGBLOCK) {                                                                                                           ./xfs_reflink.c:652:    if (count == NULLFILEOFF)                                                                                                            ./xfs_reflink.c:1462:                   if (rbno == NULLAGBLOCK)                                                                                             ./xfs_rtalloc.c:836:                    if (bp == NULL) {                                                                                                    ./xfs_rtalloc.c:899:    if (mp->m_rtdev_targp == NULL || mp->m_rbmip == NULL ||                                                                              ./xfs_rtalloc.c:1165:   if (mp->m_rtdev_targp == NULL) {                                                                                                     ./xfs_rtalloc.c:1209:   if (sbp->sb_rbmino == NULLFSINO)                                                                                                     ./xfs_trans.c:185:                      ASSERT(tp->t_ticket == NULL);                                                                                        ./xfs_trans_ail.c:59:       (prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0) &&                                                                ./xfs_trans_ail.c:60:       (next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0))                                                                  ./xfs_trans_ail.c:65:   ASSERT(prev_lsn == NULLCOMMITLSN || XFS_LSN_CMP(prev_lsn, lsn) <= 0);                                                                ./xfs_trans_ail.c:66:   ASSERT(next_lsn == NULLCOMMITLSN || XFS_LSN_CMP(next_lsn, lsn) >= 0);                                                                ./xfs_trans_ail.c:475:          if (lip == NULL)                                                                                                             ./xfs_trans_buf.c:70:   ASSERT(bp->b_transp == NULL);                                                                                                        ./xfs_trans_buf.c:155:  if (bp == NULL) {                                                                                                                    ./xfs_trans_buf.c:187:  if (tp == NULL)                                                                                                                      ./xfs_trans_buf.c:207:  if (bp == NULL)                                                                                                                      ./xfs_trans_buf.c:350:  if (tp == NULL) {                                                                                                                    ./xfs_trans_buf.c:351:          ASSERT(bp->b_transp == NULL);                                                                                                ./xfs_trans_buf.c:495:  ASSERT(bp->b_iodone == NULL ||                                                                                                       ./xfs_trans_dquot.c:103:                        if (oqa[i].qt_dquot == NULL)                                                                                 ./xfs_trans_dquot.c:149:        if (tp->t_dqinfo == NULL)                                                                                                    ./xfs_trans_dquot.c:178:                if (qa[i].qt_dquot == NULL ||                                                                                        ./xfs_trans_dquot.c:205:        if (tp->t_dqinfo == NULL)                                                                                                    ./xfs_trans_dquot.c:213:        if (qtrx->qt_dquot == NULL)                                                                                                  ./xfs_trans_dquot.c:295:        if (q[1].qt_dquot == NULL) {                                                                                                 ./xfs_trans_dquot.c:332:                if (qa[0].qt_dquot == NULL)                                                                                          ./xfs_trans_dquot.c:346:                        if ((dqp = qtrx->qt_dquot) == NULL)                                                                          ./xfs_trans_dquot.c:519:                        if ((dqp = qtrx->qt_dquot) == NULL)                                                                          ./xfs_trans_dquot.c:757:        if (tp && tp->t_dqinfo == NULL)                                                                                              ./xfs_trans_inode.c:36: if (ip->i_itemp == NULL)                                                                                                             
> ...
> 
> And in ext4, eg.
> ...
> ./mballoc.c:2892:       if (ext4_free_data_cachep == NULL) {
> ./mballoc.c:3046:       BUG_ON(lg == NULL);
> ./mballoc.c:3286:       if (pa == NULL) {
> ./mballoc.c:3377:       if (cpa == NULL) {
> ./mballoc.c:3447:       if (lg == NULL)
> ./mballoc.c:3635:       if (pa == NULL)
> ./mballoc.c:3727:       BUG_ON(ext4_pspace_cachep == NULL);
> ./mballoc.c:3729:       if (pa == NULL)
> ./mballoc.c:3756:       BUG_ON(lg == NULL);
> ./mballoc.c:4637:       BUG_ON(e4b->bd_bitmap_page == NULL);
> ./mballoc.c:4638:       BUG_ON(e4b->bd_buddy_page == NULL);
> ./move_extent.c:34:     if (path[ext_depth(inode)].p_ext == NULL) {
> ./namei.c:874:  if (frames[0].bh == NULL)
> ./namei.c:879:          if (frames[i].bh == NULL)
> ./namei.c:1432:         if ((bh = bh_use[ra_ptr++]) == NULL)
> ./page-io.c:38: if (io_end_cachep == NULL)
> ./page-io.c:398:        if (io->io_bio == NULL) {
> ./readpage.c:242:               if (bio == NULL) {
> ./resize.c:200: if (flex_gd == NULL)
> ./resize.c:210: if (flex_gd->groups == NULL)
> ./resize.c:215: if (flex_gd->bg_flags == NULL)
> ./resize.c:265: BUG_ON(flex_gd->count == 0 || group_data == NULL);
> ./resize.c:1345:        BUG_ON(flex_gd->count == 0 || group_data == NULL);
> ./resize.c:2012:        if (flex_gd == NULL) {
> ./super.c:365:  return bdi->dev == NULL;
> ./super.c:596:  if (errno == -EROFS && journal_current_handle() == NULL && sb_rdonly(sb))
> ./super.c:1086: if (ext4_inode_cachep == NULL)
> ./super.c:4050: if (sbi->s_group_desc == NULL) {
> ./super.c:4617: if (bdev == NULL)
> ./super.c:5288: if (sbi->s_journal == NULL && !(old_sb_flags & SB_RDONLY)) {
> ./xattr.c:286:  if (name == NULL)
> ./xattr.c:1905:                 if (s->base == NULL)
> ./xattr.c:1948:         if (s->base == NULL)
> ./xattr.c:2665:         if (entry == NULL) {
> ./xattr.c:2666:                 if (small_entry == NULL)
> ./xattr.c:2804: if (*ea_inode_array == NULL) {
> ./xattr.c:2813:         if (*ea_inode_array == NULL)
> ./xattr.c:2826:         if (new_array == NULL)
> ./xattr.c:2947: if (ea_inode_array == NULL)
> ...
> 
> Anyway, I'd like to listen the Greg's and Chao's ideas.

Hi Xiang,

I'm not against this change which follows checkpatch's rule, since I think this
can help to unify coding style in different modules of Linux. Maybe cleanup in
other filesystem is needed as well.

Also, personally speaking, I'm used to judge point/variable is valid or not by
using 'if {,!}{value,pointer}', it will be easy for me to know which case next
branch belongs to, just my habit being trained during f2fs development... :P

Thanks,

> 
> Thanks,
> 
>> Thanks,
>> Gao Xiang
>>

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Dan Carpenter]

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

On Fri, Aug 31, 2018 at 11:29:03AM +0800, Chao Yu wrote:
> 
> Hi Xiang,
> 
> I'm not against this change which follows checkpatch's rule, since I think this
> can help to unify coding style in different modules of Linux. Maybe cleanup in
> other filesystem is needed as well.
> 

That code is old, and those filesystems are not in staging so we're not
going to change them.

Let's just apply the patch and not spend any time thinking about it.
Part of the point of style guidelines is so that we don't have to
repeat all these discussions over and over...

Btw, I have a rename_rev.pl patch for reviewing these.  I've attached
it.  rename_rev.pl -r NULL.  I've seen some people screw up the
conversion so having an automated review is nice.

regards,
dan carpenter


[-- Attachment #2: rename_rev.pl --]
[-- Type: text/x-perl, Size: 11925 bytes --]

#!/usr/bin/perl

# This is a tool to help review variable rename patches. The goal is
# to strip out the automatic sed renames and the white space changes
# and leaves the interesting code changes.
#
# Example 1: A patch renames openInfo to open_info:
#     cat diff | rename_review.pl openInfo open_info
#
# Example 2: A patch swaps the first two arguments to some_func():
#     cat diff | rename_review.pl \
#                    -e 's/some_func\((.*?),(.*?),/some_func\($2, $1,/'
#
# Example 3: A patch removes the xkcd_ prefix from some but not all the
# variables.  Instead of trying to figure out which variables were renamed
# just remove the prefix from them all:
#     cat diff | rename_review.pl -ea 's/xkcd_//g'
#
# Example 4: A patch renames 20 CamelCase variables.  To review this let's
# just ignore all case changes and all '_' chars.
#     cat diff | rename_review -ea 'tr/[A-Z]/[a-z]/' -ea 's/_//g'
#
# The other arguments are:
# -nc removes comments
# -ns removes '\' chars if they are at the end of the line.

use strict;
use File::Temp qw/ :mktemp  /;

sub usage() {
    print "usage: cat diff | $0 old new old new old new...\n";
    print "   or: cat diff | $0 -e 's/old/new/g'\n";
    print " -a : auto";
    print " -e : execute on old lines\n";
    print " -ea: execute on all lines\n";
    print " -nc: no comments\n";
    print " -nb: no unneeded braces\n";
    print " -ns: no slashes at the end of a line\n";
    print " -pull: for function pull.  deletes context.\n";
    print " -r <recipe>: NULL, bool";
    exit(1);
}
my @subs;
my @strict_subs;
my @cmds;
my $strip_comments;
my $strip_braces;
my $strip_slashes;
my $pull_context;
my $auto;

sub filter($) {
    my $line = shift();
    my $old = 0;
    if ($line =~ /^-/) {
        $old = 1;
    }
    # remove the first char
    $line =~ s/^[ +-]//;
    if ($strip_comments) {
        $line =~ s/\/\*.*?\*\///g;
        $line =~ s/\/\/.*//;
    }
    foreach my $cmd (@cmds) {
        if ($old || $cmd->[0] =~ /^-ea$/) {
            eval "\$line =~ $cmd->[1]";
        }
    }
    foreach my $sub (@subs) {
        if ($old) {
            $line =~ s/$sub->[0]/$sub->[1]/g;
        }
    }
    foreach my $sub (@strict_subs) {
        if ($old) {
            $line =~ s/\b$sub->[0]\b/$sub->[1]/g;
        }
    }

    # remove the newline so we can move curly braces here if we want.
    $line =~ s/\n//;
    return $line;
}

while (my $param1 = shift()) {
    if ($param1 =~ /^-a$/) {
        $auto = 1;
        next;
    }
    if ($param1 =~ /^-nc$/) {
        $strip_comments = 1;
        next;
    }
    if ($param1 =~ /^-nb$/) {
        $strip_braces = 1;
        next;
    }
    if ($param1 =~ /^-ns$/) {
        $strip_slashes = 1;
        next;
    }
    if ($param1 =~ /^-pull$/) {
        $pull_context = 1;
        next;
    }
    my $param2 = shift();
    if ($param2 =~ /^$/) {
        usage();
    }
    if ($param1 =~ /^-e(a|)$/) {
        push @cmds, [$param1, $param2];
        next;
    }
    if ($param1 =~ /^-r$/) {
        if ($param2 =~ /bool/) {
            push @cmds, ["-e", "s/== true//"];
            push @cmds, ["-e", "s/true ==//"];
            push @cmds, ["-e", "s/([a-zA-Z\-\>\._]+) == false/!\$1/"];
            next;
        } elsif ($param2 =~ /NULL/) {
            push @cmds, ["-e", "s/ != NULL//"];
            push @cmds, ["-e", "s/([a-zA-Z\-\>\._0-9]+) == NULL/!\$1/"];
            next;
        } elsif ($param2 =~ /BIT/) {
            push @cmds, ["-e", 's/1[uU]* *<< *(\d+)/BIT($1)/'];
            push @cmds, ["-e", 's/\(1 *<< *(\w+)\)/BIT($1)/'];
            push @cmds, ["-e", 's/\(BIT\((.*?)\)\)/BIT($1)/'];
            next;
        }
        usage();
    }

    push @subs, [$param1, $param2];
}

my ($oldfh, $oldfile) = mkstemp("/tmp/oldXXXXX");
my ($newfh, $newfile) = mkstemp("/tmp/newXXXXX");

my @input = <STDIN>;

# auto works on the observation that the - line comes before the + line when we
# rename variables.  Take the first - line.  Find the first + line.  Find the
# one word difference.  Test that the old word never occurs in the new text.
if ($auto) {
    my %c_keywords = (  auto => 1,
                        break => 1,
                        case => 1,
                        char => 1,
                        const => 1,
                        continue => 1,
                        default => 1,
                        do => 1,
                        double => 1,
                        else => 1,
                        enum => 1,
                        extern => 1,
                        float => 1,
                        for => 1,
                        goto => 1,
                        if => 1,
                        int => 1,
                        long => 1,
                        register => 1,
                        return => 1,
                        short => 1,
                        signed => 1,
                        sizeof => 1,
                        static => 1,
                        struct => 1,
                        switch => 1,
                        typedef => 1,
                        union => 1,
                        unsigned => 1,
                        void => 1,
                        volatile => 1,
                        while => 1);
    my %old_words;
    my %new_words;
    my %added_cmds;
    my @new_subs;

    my $inside = 0;
    foreach my $line (@input) {
        if ($line =~ /^(---|\+\+\+)/) {
            next;
        }

        if ($line =~ /^@/) {
            $inside = 1;
        }
        if ($inside && !(($_ =~ /^[- @+]/) || ($_ =~ /^$/))) {
            $inside = 0;
        }
        if (!$inside) {
            next;
        }

        if ($line =~ /^-/) {
            s/-//;
            my @words = split(/\W+/, $line);
            foreach my $word (@words) {
                $old_words{$word} = 1;
            }
        } elsif ($line =~ /^\+/) {
            s/\+//;
            my @words = split(/\W+/, $line);
            foreach my $word (@words) {
                $new_words{$word} = 1;
            }
        }
    }

    my $old_line;
    my $new_line;
    $inside = 0;
    foreach my $line (@input) {
        if ($line =~ /^(---|\+\+\+)/) {
            next;
        }

        if ($line =~ /^@/) {
            $inside = 1;
        }
        if ($inside && !(($_ =~ /^[- @+]/) || ($_ =~ /^$/))) {
            $inside = 0;
        }
        if (!$inside) {
            next;
        }


        if ($line =~ /^-/ && !$old_line) {
            s/^-//;
            $old_line = $line;
            next;
        } elsif ($old_line && $line =~ /^\+/) {
            s/^\+//;
            $new_line = $line;
        } else {
            next;
        }

        my @old_words = split(/\W+/, $old_line);
        my @new_words = split(/\W+/, $new_line);
        my @new_cmds;

        my $i;
        my $diff_count = 0;
        for ($i = 0; ; $i++) {
            if (!defined($old_words[$i]) && !defined($new_words[$i])) {
                last;
            }
            if (!defined($old_words[$i]) || !defined($new_words[$i])) {
                $diff_count = 1000;
                last;
            }
            if ($old_words[$i] eq $new_words[$i]) {
                next;
            }
            if ($c_keywords{$old_words[$i]}) {
                $diff_count = 1000;
                last;
            }
            if ($new_words{$old_words[$i]}) {
                $diff_count++;
            }
            push @new_cmds, [$old_words[$i], $new_words[$i]];
        }
        if ($diff_count <= 2) {
            foreach my $sub (@new_cmds) {
                if ($added_cmds{$sub->[0] . $sub->[1]}) {
                    next;
                }
                $added_cmds{$sub->[0] . $sub->[1]} = 1;
                push @new_subs, [$sub->[0] , $sub->[1]];
            }
        }

        $old_line = 0;
    }

    if (@new_subs) {
        print "RENAMES:\n";
        foreach my $sub (@new_subs) {
            print "$sub->[0] => $sub->[1]\n";
            push @strict_subs, [$sub->[0] , $sub->[1]];
        }
        print "---\n";
    }
}

my $output;

#recreate an old file and a new file
my $stored_brace_line = "";
my $stored_remove_output = "";
my $del_close_brace = 0;
my $inside = 0;
foreach (@input) {
    my $orig = $_;


    if ($pull_context && !($orig =~ /^[+-@]/)) {
        next;
    }

    if ($orig =~ /^(---|\+\+\+)/) {
        next;
    }

    if ($orig =~ /^@/) {
        $inside = 1;
    }
    if ($inside && !(($orig =~ /^[- @+]/) || ($orig =~ /^$/))) {
        $inside = 0;
    }
    if (!$inside) {
        next;
    }

    $output = filter($orig);

    if ($strip_braces && $stored_brace_line =~ /^$/ && $orig =~ /^-(.*)\{$/) {
        $stored_brace_line = $output;
        next;
    }

    if ($strip_braces && $orig =~ /^-/ && !($stored_brace_line =~ /^$/)) {
        $stored_remove_output = $stored_remove_output . "\n" . $output;
        next;
    }

    if ($strip_braces && $del_close_brace && $orig =~ /^-\W+}\W+/) {
        $del_close_brace = 0;
        next;
    }

    if ($strip_braces && $orig =~ /^\+.*/ && !($stored_brace_line =~ /^$/)) {
        my $stripped_old = $stored_brace_line;
        my $stripped_new = $output;

        $stripped_old =~ s/(.*)\w*{/$1/;

        if ($stripped_old eq $stripped_new) {
            $orig = " ";
            $output = $stripped_new;
            $stored_brace_line = "";
            $del_close_brace = 1;
        }
    }

    if ($strip_braces && $orig =~ /^(\+|-)\W+{/) {
        $output =~ s/^[\t ]+(.*)/ $1/;
    } else {
        $output = "\n" . $output;
    }

    if ($orig =~ /^-/) {
        print $oldfh $output;
        next;
    }

    if ($orig =~ /^\+/) {
        print $newfh $output;
        next;
    }
    print $oldfh $output;
    print $newfh $output;

    if (!($stored_remove_output =~ /^$/)) {
            print $oldfh $stored_brace_line;
            print $oldfh $stored_remove_output;
            $stored_brace_line = "";
            $stored_remove_output = "";
    }
}
print $oldfh "\n";
print $newfh "\n";
# git diff puts a -- and version at the end of the diff.  put the -- into the
# new file as well so it's ignored
if ($output =~ /\n-/) {
    print $newfh "-\n";
}

my $hunk;
my $old_txt;
my $new_txt;

open diff, "diff -uw $oldfile $newfile |";
while (<diff>) {
    if ($_ =~ /^(---|\+\+\+)/) {
        next;
    }

    if ($_ =~ /^@/) {

        if ($strip_comments) {
            $old_txt =~ s/\/\*.*?\*\///g;
            $new_txt =~ s/\/\*.*?\*\///g;
        }
        if ($strip_braces) {
            $old_txt =~ s/{([^;{]*?);}/$1;/g;
            $new_txt =~ s/{([^;{]*?);}/$1;/g;
            # this is a hack because i don't know how to replace nested
            # unneeded curly braces.
            $old_txt =~ s/{([^;{]*?);}/$1;/g;
            $new_txt =~ s/{([^;{]*?);}/$1;/g;
        }

        if ($old_txt ne $new_txt) {
            print $hunk;
            print $_;
        }
        $hunk = "";
        $old_txt = "";
        $new_txt = "";
        next;
    }

    $hunk = $hunk . $_;

    if ($strip_slashes) {
        s/\\$//;
    }

    if ($_ =~ /^-/) {
        s/-//;
        s/[ \t\n]//g;
        $old_txt = $old_txt . $_;
        next;
    }
    if ($_ =~ /^\+/) {
        s/\+//;
        s/[ \t\n]//g;
        $new_txt = $new_txt . $_;
        next;
    }
    if ($_ =~ /^ /) {
        s/^ //;
        s/[ \t\n]//g;
        $old_txt = $old_txt . $_;
        $new_txt = $new_txt . $_;
    }
}

if ($old_txt ne $new_txt) {
    if ($strip_comments) {
        $old_txt =~ s/\/\*.*?\*\///g;
        $new_txt =~ s/\/\*.*?\*\///g;
    }
    if ($strip_braces) {
        $old_txt =~ s/{([^;{]*?);}/$1;/g;
        $new_txt =~ s/{([^;{]*?);}/$1;/g;
        $old_txt =~ s/{([^;{]*?);}/$1;/g;
        $new_txt =~ s/{([^;{]*?);}/$1;/g;
    }

    print $hunk;
}

# print "old = $oldfile new = $newfile\n";
unlink($oldfile);
unlink($newfile);

print "\ndone.\n";

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Chao Yu]

On 2018/8/31 17:41, Dan Carpenter wrote:
> On Fri, Aug 31, 2018 at 11:29:03AM +0800, Chao Yu wrote:
>>
>> Hi Xiang,
>>
>> I'm not against this change which follows checkpatch's rule, since I think this
>> can help to unify coding style in different modules of Linux. Maybe cleanup in
>> other filesystem is needed as well.
>>
> 
> That code is old, and those filesystems are not in staging so we're not
> going to change them.

Yup, anyway, it can be decided by their maintainer if they want the cleanup.

> 
> Let's just apply the patch and not spend any time thinking about it.
> Part of the point of style guidelines is so that we don't have to
> repeat all these discussions over and over...
> 
> Btw, I have a rename_rev.pl patch for reviewing these.  I've attached
> it.  rename_rev.pl -r NULL.  I've seen some people screw up the
> conversion so having an automated review is nice.

Cool, thanks very much for providing this tools, it actually can help to save
some time during review. :)

> 
> regards,
> dan carpenter
> 

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Gao Xiang]

Hi,

On 2018/9/3 9:59, Chao Yu wrote:
> On 2018/8/31 17:41, Dan Carpenter wrote:
>> On Fri, Aug 31, 2018 at 11:29:03AM +0800, Chao Yu wrote:
>>>
>>> Hi Xiang,
>>>
>>> I'm not against this change which follows checkpatch's rule, since I think this
>>> can help to unify coding style in different modules of Linux. Maybe cleanup in
>>> other filesystem is needed as well.
>>>
>>
>> That code is old, and those filesystems are not in staging so we're not
>> going to change them.
> 
> Yup, anyway, it can be decided by their maintainer if they want the cleanup.
> 
>>
>> Let's just apply the patch and not spend any time thinking about it.
>> Part of the point of style guidelines is so that we don't have to
>> repeat all these discussions over and over...
>>
>> Btw, I have a rename_rev.pl patch for reviewing these.  I've attached
>> it.  rename_rev.pl -r NULL.  I've seen some people screw up the
>> conversion so having an automated review is nice.
> 
> Cool, thanks very much for providing this tools, it actually can help to save
> some time during review. :)
> 

I am traveling in Tibet. Sorry for the late response.

OK, that is fine. I will follow the new kernel coding style strictly
for the upcoming code.

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

>>
>> regards,
>> dan carpenter
>>
> 

Re: [PATCH 2/6] staging: erofs: formatting fix to NULL comparison

[Chao Yu]

On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch checks by
> to NULL comparison.
> 
> The patch fixes 5 checks of type:
> "Comparison to NULL could be written".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

[PATCH 3/6] staging: erofs: formatting spaces around '-'

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch checks by
adding spaces around '-'.

The patch fixes 4 checks of type:
"Check: spaces preferred around that '-'".

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index 54da72a..bf1f51c 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -65,13 +65,13 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 
 		if (i) {
 			if (!src)
-				src = mirrored[i-1] ?
-					percpu_data + (i-1) * PAGE_SIZE :
-					kmap_atomic(compressed_pages[i-1]);
+				src = mirrored[i - 1] ?
+					percpu_data + (i - 1) * PAGE_SIZE :
+					kmap_atomic(compressed_pages[i - 1]);
 
 			memcpy(dst, src + righthalf, pageofs);
 
-			if (!mirrored[i-1])
+			if (!mirrored[i - 1])
 				kunmap_atomic(src);
 
 			if (unlikely(i >= clusterpages)) {
-- 
2.7.4

Re: [PATCH 3/6] staging: erofs: formatting spaces around '-'

[Chao Yu]

On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch checks by
> adding spaces around '-'.
> 
> The patch fixes 4 checks of type:
> "Check: spaces preferred around that '-'".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 3/6] staging: erofs: formatting spaces around '-'

[Gao Xiang]

On 2018/9/3 9:48, Chao Yu wrote:
> On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch checks by
>> adding spaces around '-'.
>>
>> The patch fixes 4 checks of type:
>> "Check: spaces preferred around that '-'".
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> Thanks,
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> 

[PATCH 4/6] staging: erofs: formatting add spaces arround '*'

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch check by
adding spaces around '*'.

The patch fixes 1 check of type:
"Check: spaces preferred around that '*'".

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index bf1f51c..7389500 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -181,7 +181,7 @@ int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
 		for (i = 0; i < clusterpages; ++i) {
 			void *t = kmap_atomic(compressed_pages[i]);
 
-			memcpy(vin + PAGE_SIZE *i, t, PAGE_SIZE);
+			memcpy(vin + PAGE_SIZE * i, t, PAGE_SIZE);
 			kunmap_atomic(t);
 		}
 	} else if (clusterpages == 1)
-- 
2.7.4

Re: [PATCH 4/6] staging: erofs: formatting add spaces arround '*'

[Chao Yu]

On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch check by
> adding spaces around '*'.
> 
> The patch fixes 1 check of type:
> "Check: spaces preferred around that '*'".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 4/6] staging: erofs: formatting add spaces arround '*'

[Gao Xiang]

On 2018/9/3 9:48, Chao Yu wrote:
> On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch check by
>> adding spaces around '*'.
>>
>> The patch fixes 1 check of type:
>> "Check: spaces preferred around that '*'".
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> Thanks,
> 

[PATCH 5/6] staging: erofs: formatting alignment parenthesis

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch check by
alignment should match open parenthesis.

The patch fixes 2 check of type:
"Check: Alignment should match open parenthesis".

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index 7389500..f285a50 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -126,7 +126,7 @@ int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
 	vout = erofs_pcpubuf[smp_processor_id()].data;
 
 	ret = z_erofs_unzip_lz4(vin, vout + pageofs,
-		clusterpages * PAGE_SIZE, outlen);
+				clusterpages * PAGE_SIZE, outlen);
 
 	if (ret >= 0) {
 		outlen = ret;
@@ -191,7 +191,7 @@ int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
 	}
 
 	ret = z_erofs_unzip_lz4(vin, vout + pageofs,
-		clusterpages * PAGE_SIZE, llen);
+				clusterpages * PAGE_SIZE, llen);
 	if (ret > 0)
 		ret = 0;
 
-- 
2.7.4

Re: [PATCH 5/6] staging: erofs: formatting alignment parenthesis

[Chao Yu]

On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch check by
> alignment should match open parenthesis.
> 
> The patch fixes 2 check of type:
> "Check: Alignment should match open parenthesis".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 5/6] staging: erofs: formatting alignment parenthesis

[Gao Xiang]

On 2018/9/3 9:49, Chao Yu wrote:
> On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch check by
>> alignment should match open parenthesis.
>>
>> The patch fixes 2 check of type:
>> "Check: Alignment should match open parenthesis".
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> Thanks,
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> 

[PATCH 6/6] staging: erofs: fix 1 warning and 9 checks

[Pavel Zemlyanoy]

This patch does not change the logic, it only
corrects the formatting and checkpatch checks by
braces {} should be used on all arms of this statement,
unbalanced braces around else statement and warning by
braces {} are not necessary for any arm of this statement.

The patch fixes 9 checks of type:
"Check: braces {} should be used on all arms of this statement";
"Check: Unbalanced braces around else statement";

and 1 warning of type:
"WARNING: braces {} are not necessary for any arm of this statement".

Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
---
 drivers/staging/erofs/unzip_vle_lz4.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/erofs/unzip_vle_lz4.c b/drivers/staging/erofs/unzip_vle_lz4.c
index f285a50..1a42865 100644
--- a/drivers/staging/erofs/unzip_vle_lz4.c
+++ b/drivers/staging/erofs/unzip_vle_lz4.c
@@ -80,9 +80,9 @@ int z_erofs_vle_plain_copy(struct page **compressed_pages,
 			}
 		}
 
-		if (!righthalf)
+		if (!righthalf) {
 			src = NULL;
-		else {
+		} else {
 			src = mirrored[i] ? percpu_data + i * PAGE_SIZE :
 				kmap_atomic(compressed_pages[i]);
 
@@ -137,11 +137,12 @@ int z_erofs_vle_unzip_fast_percpu(struct page **compressed_pages,
 		j = min((unsigned int)PAGE_SIZE - pageofs, outlen);
 
 		if (pages[i]) {
-			if (ret < 0)
+			if (ret < 0) {
 				SetPageError(pages[i]);
-			else if (clusterpages == 1 && pages[i] == compressed_pages[0])
+			} else if (clusterpages == 1 &&
+				   pages[i] == compressed_pages[0]) {
 				memcpy(vin + pageofs, vout + pageofs, j);
-			else {
+			} else {
 				void *dst = kmap_atomic(pages[i]);
 
 				memcpy(dst + pageofs, vout + pageofs, j);
@@ -184,9 +185,9 @@ int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
 			memcpy(vin + PAGE_SIZE * i, t, PAGE_SIZE);
 			kunmap_atomic(t);
 		}
-	} else if (clusterpages == 1)
+	} else if (clusterpages == 1) {
 		vin = kmap_atomic(compressed_pages[0]);
-	else {
+	} else {
 		vin = erofs_vmap(compressed_pages, clusterpages);
 	}
 
@@ -198,11 +199,10 @@ int z_erofs_vle_unzip_vmap(struct page **compressed_pages,
 	if (!overlapped) {
 		if (clusterpages == 1)
 			kunmap_atomic(vin);
-		else {
+		else
 			erofs_vunmap(vin, clusterpages);
-		}
-	} else
+	} else {
 		preempt_enable();
-
+	}
 	return ret;
 }
-- 
2.7.4

Re: [PATCH 6/6] staging: erofs: fix 1 warning and 9 checks

[Chao Yu]

On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
> This patch does not change the logic, it only
> corrects the formatting and checkpatch checks by
> braces {} should be used on all arms of this statement,
> unbalanced braces around else statement and warning by
> braces {} are not necessary for any arm of this statement.
> 
> The patch fixes 9 checks of type:
> "Check: braces {} should be used on all arms of this statement";
> "Check: Unbalanced braces around else statement";
> 
> and 1 warning of type:
> "WARNING: braces {} are not necessary for any arm of this statement".
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 6/6] staging: erofs: fix 1 warning and 9 checks

[Gao Xiang]

On 2018/9/3 9:49, Chao Yu wrote:
> On 2018/8/30 23:14, Pavel Zemlyanoy wrote:
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch checks by
>> braces {} should be used on all arms of this statement,
>> unbalanced braces around else statement and warning by
>> braces {} are not necessary for any arm of this statement.
>>
>> The patch fixes 9 checks of type:
>> "Check: braces {} should be used on all arms of this statement";
>> "Check: Unbalanced braces around else statement";
>>
>> and 1 warning of type:
>> "WARNING: braces {} are not necessary for any arm of this statement".
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> Thanks,
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
> 

Re: [PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c

[Chao Yu]

On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
> 
> This patch does not change the logic, it only
> corrects the formatting and checkpatch warnings by
> adding "int" to the unsigned type.
> 
> The patch fixes 11 warnings of the type:
> "WARNING: Prefer 'unsigned int' to bare use of 'unsigned'"
> 
> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 1/6] staging: erofs: formatting fix in unzip_vle_lz4.c

[Gao Xiang]

On 2018/9/3 9:47, Chao Yu wrote:
> On 2018/8/30 23:13, Pavel Zemlyanoy wrote:
>>
>> This patch does not change the logic, it only
>> corrects the formatting and checkpatch warnings by
>> adding "int" to the unsigned type.
>>
>> The patch fixes 11 warnings of the type:
>> "WARNING: Prefer 'unsigned int' to bare use of 'unsigned'"
>>
>> Signed-off-by: Pavel Zemlyanoy <zemlyanoy@ispras.ru>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 

Reviewed-by: Gao Xiang <gaoxiang25@huawei.com>

Thanks,
Gao Xiang

> Thanks,
> 
> _______________________________________________
> devel mailing list
> devel@linuxdriverproject.org
> http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
>