Skip to content

Commit

Permalink
eval: Flush buffer in write_list
Browse files Browse the repository at this point in the history
This way success/failure return from this function is more precise.
  • Loading branch information
ZyX-I committed Feb 22, 2017
1 parent 0086991 commit 31cdb22
Showing 1 changed file with 3 additions and 0 deletions.
3 changes: 3 additions & 0 deletions src/nvim/eval.c
Expand Up @@ -18069,6 +18069,9 @@ static bool write_list(FileDescriptor *const fp, const list_T *const list,
}
}
}
if ((error = file_fsync(fp)) != 0) {
goto write_list_error;
}
return true;
write_list_error:
emsgf(_("E80: Error while writing: %s"), os_strerror(error));
Expand Down

5 comments on commit 31cdb22

@blueyed
Copy link
Contributor

@blueyed blueyed commented on 31cdb22 Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZyX-I
This causes a slowdown in Docker for me (Alpine Linux 3.5).
Any idea why?
Will file an issue for it later anyway.

@ZyX-I
Copy link
Contributor Author

@ZyX-I ZyX-I commented on 31cdb22 Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do not know. file_fsync is a thing which will be done in any case, when freeing fp.

@blueyed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZyX-I
So should I test it with just this change reverted then?

@ZyX-I
Copy link
Contributor Author

@ZyX-I ZyX-I commented on 31cdb22 Mar 27, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think yes, I do not see any reasons for this change to cause any problems, except for obvious missing of error return (errorred file_free which also does file_fsync causes error message, but returned number is still zero), which AFAIR is not even tested.

@blueyed
Copy link
Contributor

@blueyed blueyed commented on 31cdb22 Apr 1, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ZyX-I
Followed up in #6420.

Please sign in to comment.