[codehealth] remove some unused stuff #2797
Conversation
crypto/des/rpc_enc.c
Outdated
| @@ -1,30 +0,0 @@ | |||
| /* | |||
There was a problem hiding this comment.
This is being removed in #2776.
There was a problem hiding this comment.
Great minds think alike, I guess :)
(BTW, while I was looking at my old notes about crufty things in the tree, I ran into OPENSSL_indirect_call(), which appears to be undocumented and unused in the tree, but it has also been that way for a very long time, so I am more inclined to believe that I am misreading how it is used than that it is safe to remove.)
There was a problem hiding this comment.
OPENSSL_indirect_call(), which appears to be undocumented
See commentary in crypto/x86cpuid.pl. Original idea was to use it with 3rd party DLLs, those that would be loaded at run-time with LoadLibrary, without having to think about calling convention.
and unused in the tree
That's correct. Idea got no traction. Yeah, it can be removed...
There was a problem hiding this comment.
Idea got no traction. Yeah, it can be removed...
Do you want to make the pull request or should I?
There was a problem hiding this comment.
Go ahead... [Just in case, I'm signing off for today in few minutes.]
|
But removal of rpc_enc should be skipped, #2776 does it more thoroughly. |
Some things were not removed from util/indent.pro when they were removed
from the code.
grep '^-T' util/indent.pro | awk '{print $2} > /tmp/a
grep -rF -f /tmp/a --exclude CHANGES --exclude 'INSTALL' --exclude 'LICENSE' --exclude 'NEWS' --exclude 'NOTES*' --exclude 'README*' --exclude indent.pro --exclude-dir corpora -o -h *|sort|uniq>/tmp/b
comm -23 <(sort /tmp/a) /tmp/b >/tmp/c
grep -v -E '(LHASH_OF|STACK_OF)' /tmp/c > /tmp/d
grep -v -Ff /tmp/d util/indent.pro > util/indent.pro
Manually adjusted to retain time_t and the ossl_*intmax_t types.
It's even removing a BUGS entry!
|
Technically this is an API change even though the PEM structures aren't actually used anywhere. I guess the probability of someone actually trying to declare one of these things is close to zero though?? |
Yes. (Do we even promise API compatibility or just ABI?) |
Yes - it must be source and ABI compatible. |
|
I'd like other team member opinions on whether this change is ok or not. |
|
I am in favor of this. I doubt anyone is using these structures and, if they are, we can treat it like the opaque wolrk and restore them in a bugfix. |
|
I will plus-one this if nobody else does, but I'd rather it was from someone else. |
|
I'm ambivalent... If you want me to make a difference, give me a couple more hours |
|
I bow to you |
ekasper
added
the
approval: done
Reviewed-by: Emilia Käsper <emilia@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #2797)
Some things were not removed from util/indent.pro when they were removed
from the code.
grep '^-T' util/indent.pro | awk '{print $2} > /tmp/a
grep -rF -f /tmp/a --exclude CHANGES --exclude 'INSTALL' --exclude 'LICENSE' --exclude 'NEWS' --exclude 'NOTES*' --exclude 'README*' --exclude indent.pro --exclude-dir corpora -o -h *|sort|uniq>/tmp/b
comm -23 <(sort /tmp/a) /tmp/b >/tmp/c
grep -v -E '(LHASH_OF|STACK_OF)' /tmp/c > /tmp/d
grep -v -Ff /tmp/d util/indent.pro > util/indent.pro
Manually adjusted to retain time_t and the ossl_*intmax_t types.
Reviewed-by: Emilia Käsper <emilia@openssl.org>
Reviewed-by: Rich Salz <rsalz@openssl.org>
Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from #2797)
It's even removing a BUGS entry! Reviewed-by: Emilia Käsper <emilia@openssl.org> Reviewed-by: Rich Salz <rsalz@openssl.org> Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from #2797)
Description of change
There's probably some more low-hanging fruit in util/indent.pro and crypto/des, but I am about to be called over to something else.