RSA: deal with openssl.cfg better #286
Conversation
| $this->configFile = CRYPT_RSA_OPENSSL_CONFIG; | ||
| $dir = '@cfg_dir@' . DIRECTORY_SEPARATOR . 'Crypt_RSA'; | ||
| if (strpos($dir, '@') === false) { | ||
| // PEAR installer was used to install the package |
There was a problem hiding this comment.
I noticed that in the orig commit but decided to keep it. The alternative would have a comment that says something like "if this condition wasn't met the PEAR installer was used to install the package", which is more wordy and if you're not careful you could miss the "n't" and think it was saying the exact opposite of what it really was.
There was a problem hiding this comment.
You could also interpret all sort of other things in a different way. Of course you have to read carefully and correctly, but this is a rather basic assumption. There should be proper comments and there should be no empty blocks. Having empty blocks is just ridicolous.
There was a problem hiding this comment.
I wouldn't say it was empty lol. There's no code there but there is a comment that satisfies the whole if / then paradigm. And from a performance perspective idk that it's any worse than a !==.
I don't know that I would have thought to have done it that way but unless it's violating a coding standard or it's inconsistent with some other established way of doing things (eg. using fsockopen vs. socket_create; a departure I made from the orig ssh-agent PR) idk that I care that much.
/shrug/ I guess you can change it if you want to idk.
|
See #272 (comment) |
|
I don't think we should have this patch before the actual issue that is being fixed is explained in more detail. See #272 (comment) |
Fixes #272