fuse_writepages_fill: simplified "if-else if" constuction
diff mbox series

Message ID 446f0df5-798d-ab3a-e773-39d9f202c092@virtuozzo.com
State New
Headers show
Series
  • fuse_writepages_fill: simplified "if-else if" constuction
Related show

Commit Message

Vasily Averin June 25, 2020, 9:30 a.m. UTC
fuse_writepages_fill uses following construction:
if (wpa && ap->num_pages &&
    (A || B || C)) {
	action;
} else if (wpa && D) {
	if (E) {
		the same action;
	}
}

- ap->num_pages check is always true and can be removed
- "if" and "else if" calls the same action and can be merged.

Signed-off-by: Vasily Averin <vvs@virtuozzo.com>
---
 fs/fuse/file.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

Comments

Miklos Szeredi July 14, 2020, 12:24 p.m. UTC | #1
On Thu, Jun 25, 2020 at 11:30 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>
> fuse_writepages_fill uses following construction:
> if (wpa && ap->num_pages &&
>     (A || B || C)) {
>         action;
> } else if (wpa && D) {
>         if (E) {
>                 the same action;
>         }
> }
>
> - ap->num_pages check is always true and can be removed
> - "if" and "else if" calls the same action and can be merged.

Makes sense.  Attached patch goes further and moves checking the
conditions to a separate helper for clarity.

Thanks,
Miklos
Vasily Averin July 14, 2020, 6:53 p.m. UTC | #2
On 7/14/20 3:24 PM, Miklos Szeredi wrote:
> On Thu, Jun 25, 2020 at 11:30 AM Vasily Averin <vvs@virtuozzo.com> wrote:
>>
>> fuse_writepages_fill uses following construction:
>> if (wpa && ap->num_pages &&
>>     (A || B || C)) {
>>         action;
>> } else if (wpa && D) {
>>         if (E) {
>>                 the same action;
>>         }
>> }
>>
>> - ap->num_pages check is always true and can be removed
>> - "if" and "else if" calls the same action and can be merged.
> 
> Makes sense.  Attached patch goes further and moves checking the
> conditions to a separate helper for clarity.

This looks perfect for me, thank you
	Vasily Averin

Patch
diff mbox series

diff --git a/fs/fuse/file.c b/fs/fuse/file.c
index cf267bd..c023f7f0 100644
--- a/fs/fuse/file.c
+++ b/fs/fuse/file.c
@@ -2035,17 +2035,14 @@  static int fuse_writepages_fill(struct page *page,
 	 */
 	is_writeback = fuse_page_is_writeback(inode, page->index);
 
-	if (wpa && ap->num_pages &&
+	if (wpa &&
 	    (is_writeback || ap->num_pages == fc->max_pages ||
 	     (ap->num_pages + 1) * PAGE_SIZE > fc->max_write ||
-	     data->orig_pages[ap->num_pages - 1]->index + 1 != page->index)) {
+	     (data->orig_pages[ap->num_pages - 1]->index + 1 != page->index) ||
+	     ((ap->num_pages == data->max_pages) &&
+				(!fuse_pages_realloc(data))))) {
 		fuse_writepages_send(data);
 		data->wpa = NULL;
-	} else if (wpa && ap->num_pages == data->max_pages) {
-		if (!fuse_pages_realloc(data)) {
-			fuse_writepages_send(data);
-			data->wpa = NULL;
-		}
 	}
 
 	err = -ENOMEM;