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

SLE-11 SP1: Use stdin, not tmp files for passwords (bsc#986971) #226

Merged
merged 2 commits into from
Aug 31, 2016
Merged

SLE-11 SP1: Use stdin, not tmp files for passwords (bsc#986971) #226

merged 2 commits into from
Aug 31, 2016

Conversation

shundhammer
Copy link
Contributor

Backport to SLE-11 SP1.

Since there were a lot of changes between SP1 and SP3, the SP3/SP4 patch did not apply at all, so this is very much a manual effort.

Still, it is quite straightforward to compare this PR manually to the PR against SP4:

https://github.com/yast/yast-storage/pull/223/files

I also created RPMs in the IBS with this version, built a DUD, integrated that DUD into the SLES-11-SP1 ISO and tested with that ISO: It installs without problem, and the created encrypted LVM can successfully be decrypted so that system boots without any problems.

@@ -256,6 +288,11 @@ SystemCmd::doExecute( string Cmd )
y2error( "dup2 stderr child failed errno=%d (%s)", errno,
strerror(errno));
}
if( close( sin[1] )<0 )
{
y2error( "close child failed errno=%d (%s)", errno,
Copy link
Contributor

Choose a reason for hiding this comment

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

NP (I wouldn't change already tested code for this): the original patch used "close (stdin) child failer errno", which is probably convenient for easily distinguish from the error closing sout below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, but I wanted to keep it as similar to the surrounding code as possible -- and there it's similarly impossible to tell which close() went wrong - stdout or stderr? Now it's one more to guess from. But when that happens, something went badly wrong already, so it's just cosmetic which of those close() calls failed.

@ancorgs
Copy link
Contributor

ancorgs commented Aug 31, 2016

After a close inspection, I confirm this patch looks 100% equivalent to the SP4 one, so LGTM.

@lslezak
Copy link
Member

lslezak commented Aug 31, 2016

I have compared it to the https://github.com/yast/yast-storage/pull/223/files change and nothing was forgotten. So again, small LGTM from me 😉

@shundhammer shundhammer merged commit 671ba4f into yast:Code-11-SP1 Aug 31, 2016
@shundhammer shundhammer deleted the huha-pipe-systemcmd-SLE-11-SP1 branch September 14, 2016 10: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.

None yet

3 participants