Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update readpassphrase #425

Closed
wants to merge 1 commit into from
Closed

update readpassphrase #425

wants to merge 1 commit into from

Conversation

vgough
Copy link
Owner

@vgough vgough commented Oct 13, 2017

@benrubson This updates readpassphrase to the latest version I've found in a quick search. I made a couple changes over the original:

  • build as C++
  • adjust include, add path.h include
  • change NULL -> nullptr
  • reformat with clang-format

@benrubson
Copy link
Contributor

benrubson commented Oct 13, 2017

Clang-tidy complains a lot regarding this new version :)
And I think it should also complain (once some warnings will have been corrected ?) about the 2 (void)write(...).

@vgough
Copy link
Owner Author

vgough commented Oct 15, 2017

There are still lots of clang-tidy complaints throughout encfs. Looks like most of the complaints in readpassphrase are readability-implicit-bool-cast and readability-braces-around-statements, neither of which bother me much for imported C code.

I don't see any clang-tidy or compiler complaints about (void)write, so not sure what you're expecting.

@benrubson
Copy link
Contributor

benrubson commented Oct 15, 2017

Sorry, these (void)write warnings are not triggered by clang but by gcc... My mistake...
Taken from log given in #407 :

/home/makerpm/rpmbuild/BUILD/encfs-master/encfs/readpassphrase.cpp:128:46: warning: ignoring return value of 'ssize_t write(int, const void*, size_t)', declared with attribute warn_unused_result [-Wunused-result]
   (void)write(output, prompt, strlen(prompt));
                                              ^
/home/makerpm/rpmbuild/BUILD/encfs-master/encfs/readpassphrase.cpp:149:33: warning: ignoring return value of 'ssize_t write(int, const void*, size_t)', declared with attribute warn_unused_result [-Wunused-result]
     (void)write(output, "\n", 1);
                                 ^

I then would like to correct this, so that these warnings do not bother users anymore.
Thus I opened PR #424 (which I just updated with the few log lines above to clarify).

For sure clang-tidy complaints regarding the new readpassphrase version are not so important, but I think it would be interesting to have clang-tidy to warn as less as possible, to easily detect future code mistakes. This is why I opened #427 (as well as #426), to try to make an effort to correct clang-tidy & compilation warnings.

Many thanks 👍

@samrocketman
Copy link
Collaborator

@vgough this PR would be more appropriate as two commits because you're copying code. 1 commit for the original and 1 commit for your explicit changes. This way your changes are more obviously documented because I can't tell what is your changes and what is the original copy at first glance.

@vgough vgough closed this Oct 19, 2017
@vgough vgough deleted the readpass-update branch October 19, 2017 23:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants