On Fri, Mar 17, 2017 at 9:04 PM, <hpa@xxxxxxxxx> wrote: > On March 17, 2017 12:27:46 PM PDT, Peter Zijlstra <peterz@xxxxxxxxxxxxx> 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 >><peterz@xxxxxxxxxxxxx> wrote: >>> > >>> > Be that as it may; what you construct above is disgusting. Surely >>the >>> > 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 >>somewhere. >>> > >>> >>> 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 >>a >>> 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 >>whoever >>> might touch the code next. >>> >>> I believe that the actual stack usage is exactly the same as it was >>previously. >>> >>> 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". https://lkml.org/lkml/2013/9/23/500 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. -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
