On Fri, Mar 17, 2017 at 9:04 PM,  <h...@zytor.com> wrote:
> On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <pet...@infradead.org> 
> wrote:
>>On Fri, Mar 17, 2017 at 11:52:01AM -0700, Michael Davidson wrote:
>>> On Fri, Mar 17, 2017 at 5:44 AM, Peter Zijlstra
>><pet...@infradead.org> wrote:
>>> >
>>> > Be that as it may; what you construct above is disgusting. Surely
>>> > code can be refactored to not look like dog vomit?
>>> >
>>> > Also; its not immediately obvious conf->copies is 'small' and this
>>> > doesn't blow up the stack; I feel that deserves a comment
>>> >
>>> I agree that the code is horrible.
>>> It is, in fact, exactly the same solution that was used to remove
>>> variable length arrays in structs from several of the crypto drivers
>>> few years ago - see the definition of SHASH_DESC_ON_STACK() in
>>> "crypto/hash.h" - I did not, however, hide the horrors in a macro
>>> preferring to leave the implementation visible as a warning to
>>> might touch the code next.
>>> I believe that the actual stack usage is exactly the same as it was
>>> I can certainly wrap this  up in a macro and add comments with
>>> appropriately dire warnings in it if you feel that is both necessary
>>> and sufficient.
>>We got away with ugly in the past, so we should get to do it again?
> Seriously, you should have taken the hack the first time that this needs to 
> be fixed.  Just because this is a fairly uncommon construct in the kernel 
> doesn't mean it is not in userspace.

There is a reason why it is fairly uncommon in kernel.
Initially it was used more widely, but then there was a decision to
drop all uses of this feature. Namely:
Linus: "We should definitely drop it. The feature is an abomination".
I really don't understand why you cling onto this last use of the
feature. Having a single use of a compiler extension on an error path
of a non-mandatory driver does not look like a great idea to me. Let's
just kill it off outside of clang discussion.

Reply via email to