From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on archive.lwn.net X-Spam-Level: X-Spam-Status: No, score=-9.7 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,FSL_HELO_FAKE,HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_HI,SPF_HELO_NONE,SPF_NONE, T_DKIMWL_WL_MED,USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by archive.lwn.net (Postfix) with ESMTP id 5EDE17D2EF for ; Tue, 11 Jun 2019 17:58:38 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2406717AbfFKR6i (ORCPT ); Tue, 11 Jun 2019 13:58:38 -0400 Received: from mail-pl1-f193.google.com ([209.85.214.193]:44816 "EHLO mail-pl1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S2406713AbfFKR6h (ORCPT ); Tue, 11 Jun 2019 13:58:37 -0400 Received: by mail-pl1-f193.google.com with SMTP id t7so2822581plr.11 for ; Tue, 11 Jun 2019 10:58:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=ezWWyHmFr3D52R1hMjM3lFqjTF5qDUt/Gz8+w6Ybeeg=; b=NXabX8VKhqlLINuoMQHbinmJIUbVH0BN+gV3EBZfeU2WCiSZGaL5z1BBk/58lNpSLk 6OZS/QBr9Xcqp8FLP0aoFzarQalnNz5o/W4LmU3plkkiz8px1w/b14QGZtx/LpELMViT Cq0zGOdkG0bUXDxO/omwJpd2ykSJkUq6SnLNsOdyke4UAJxvvy6sJ2KCrvAjbWxYUAfv mNi2NE/pq7hdDfUErhLVHQrN935jeqQID0v5d+Q6AZHeXiOFfmRa4pwDvFIRPShgom9k +iiaBa56wRxxqOBXtYUg69RYV0vztcjvfpfAUcRMjGUqaocx9uX+ixG798K/4fglHZBe neug== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=ezWWyHmFr3D52R1hMjM3lFqjTF5qDUt/Gz8+w6Ybeeg=; b=nQGJfgq34dvScrE/zdwI1XFRRubWt/CaYu51PrnTj463+yehfO5Z54Id7EVde3x+5y QkiZSfiiH/rMMIU9fe8b3E7zBDuQzjtu5Frmo4VcAyDBTq45V0Uerno62Q6CTNoB5SnR aHQIotmvVPqIuaakHrmBmApwqLH7wxxGvKah2evz3RSgqv6wo392PN4FxpvW50YEO1wg Daenh8izkKoHtiCEF+iV6J6JbF4zOAx/r4XhlH0QJXTyzDwcIEEZsJ2KRNXKBZ6Jcn8H 2W/g4VQKyoapg50QHw6TzOlF92XlwbWEJrzvLqmbhTM1426lJ8Hco9m6mfNp7HHRnxoQ LzBg== X-Gm-Message-State: APjAAAXHmTPKyYfZWOQ1pP2EmjyUw+Mc/q+i1EtJ42IdVWtz/1CO4faP C8+hcAUj0FawnMc4UiehjH0ybA== X-Google-Smtp-Source: APXvYqxLhXRMoy3niXLUf+3yjcR+oYOqDZdO4IrAaXbVjtaTgohrWX6P7B3Tnf00xa75XTTIUTknBA== X-Received: by 2002:a17:902:3341:: with SMTP id a59mr47725400plc.186.1560275916197; Tue, 11 Jun 2019 10:58:36 -0700 (PDT) Received: from google.com ([2620:15c:2cd:2:d714:29b4:a56b:b23b]) by smtp.gmail.com with ESMTPSA id s64sm13222982pfb.160.2019.06.11.10.58.34 (version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256); Tue, 11 Jun 2019 10:58:35 -0700 (PDT) Date: Tue, 11 Jun 2019 10:58:30 -0700 From: Brendan Higgins To: Stephen Boyd Cc: Iurii Zaikin , frowand.list@gmail.com, gregkh@linuxfoundation.org, jpoimboe@redhat.com, keescook@google.com, kieran.bingham@ideasonboard.com, mcgrof@kernel.org, peterz@infradead.org, robh@kernel.org, shuah@kernel.org, tytso@mit.edu, yamada.masahiro@socionext.com, devicetree@vger.kernel.org, dri-devel@lists.freedesktop.org, kunit-dev@googlegroups.com, linux-doc@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org, linux-nvdimm@lists.01.org, linux-um@lists.infradead.org, Alexander.Levin@microsoft.com, Tim.Bird@sony.com, amir73il@gmail.com, dan.carpenter@oracle.com, daniel@ffwll.ch, jdike@addtoit.com, joel@jms.id.au, julia.lawall@lip6.fr, khilman@baylibre.com, knut.omang@oracle.com, logang@deltatee.com, mpe@ellerman.id.au, pmladek@suse.com, rdunlap@infradead.org, richard@nod.at, rientjes@google.com, rostedt@goodmis.org, wfg@linux.intel.com Subject: Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec() Message-ID: <20190611175830.GA236872@google.com> References: <20190514221711.248228-1-brendanhiggins@google.com> <20190514221711.248228-18-brendanhiggins@google.com> <20190517182254.548EA20815@mail.kernel.org> <20190607190047.C3E7A20868@mail.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-doc-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Fri, Jun 07, 2019 at 12:00:47PM -0700, Stephen Boyd wrote: > Quoting Iurii Zaikin (2019-06-05 18:29:42) > > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd wrote: > > > > > > Quoting Brendan Higgins (2019-05-14 15:17:10) > > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c > > > > new file mode 100644 > > > > index 0000000000000..fe0f2bae66085 > > > > --- /dev/null > > > > +++ b/kernel/sysctl-test.c > > > > + > > > > + > > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test) > > > > +{ > > > > + struct ctl_table table = { > > > > + .procname = "foo", > > > > + .data = &test_data.int_0001, > > > > + .maxlen = sizeof(int), > > > > + .mode = 0644, > > > > + .proc_handler = proc_dointvec, > > > > + .extra1 = &i_zero, > > > > + .extra2 = &i_one_hundred, > > > > + }; > > > > + char input[] = "-9"; > > > > + size_t len = sizeof(input) - 1; > > > > + loff_t pos = 0; > > > > + > > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER); > > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos)); > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len); > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos); > > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data); > > > > > > Is the casting necessary? Or can the macro do a type coercion of the > > > second parameter based on the first type? > > Data field is defined as void* so I believe casting is necessary to > > dereference it as a pointer to an array of ints. I don't think the > > macro should do any type coercion that == operator wouldn't do. > > I did change the cast to make it more clear that it's a pointer to an > > array of ints being dereferenced. > > Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on > both sides and cause a build warning/error if the types aren't the same. > This would be similar to our min/max macros that complain about > mismatched types in the comparisons. Then if a test developer needs to > convert one type or the other they could do so with a > KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to > explicitly. Do you think it would be better to do a phony compare similar to how min/max used to work prior to 4.17, or to use the new __typecheck(...) macro? This might seem like a dumb question (and maybe it is), but Iurii and I thought the former created an error message that was a bit easier to understand, whereas __typecheck is obviously superior in terms of code reuse. This is what we are thinking right now; if you don't have any complaints I will squash it into the relevant commits on the next revision: --- From: Iurii Zaikin Adds a warning message when comparing values of different types similar to what min() / max() macros do. Signed-off-by: Iurii Zaikin --- include/kunit/test.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/include/kunit/test.h b/include/kunit/test.h index 511c9e85401a6..791e22fba5620 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -335,6 +335,13 @@ void __printf(3, 4) kunit_printk(const char *level, #define kunit_err(test, fmt, ...) \ kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__) +/* + * 'Unnecessary' cast serves to generate a compile-time warning in case + * of comparing incompatible types. Inspired by include/linux/kernel.h + */ +#define __kunit_typecheck(lhs, rhs) \ + ((void) (&(lhs) == &(rhs))) + static inline struct kunit_stream *kunit_expect_start(struct kunit *test, const char *file, const char *line) @@ -514,6 +521,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_binary(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -524,6 +532,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_binary_msg(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -538,6 +547,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_ptr_binary(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -553,6 +563,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_ptr_binary_msg(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -1013,6 +1024,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_binary(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -1023,6 +1035,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_binary_msg(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -1037,6 +1050,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_PTR_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_ptr_binary(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -1051,6 +1065,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_ptr_binary_msg(test, \ (void *) __left, #left, \ (void *) __right, #right, \ -- 2.22.0.rc2.383.gf4fbbf30c2-goog From mboxrd@z Thu Jan 1 00:00:00 1970 From: Brendan Higgins Subject: Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec() Date: Tue, 11 Jun 2019 10:58:30 -0700 Message-ID: <20190611175830.GA236872@google.com> References: <20190514221711.248228-1-brendanhiggins@google.com> <20190514221711.248228-18-brendanhiggins@google.com> <20190517182254.548EA20815@mail.kernel.org> <20190607190047.C3E7A20868@mail.kernel.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Stephen Boyd Cc: pmladek@suse.com, linux-doc@vger.kernel.org, peterz@infradead.org, amir73il@gmail.com, dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com, yamada.masahiro@socionext.com, mpe@ellerman.id.au, linux-kselftest@vger.kernel.org, shuah@kernel.org, linux-nvdimm@lists.01.org, frowand.list@gmail.com, knut.omang@oracle.com, kieran.bingham@ideasonboard.com, wfg@linux.intel.com, joel@jms.id.au, rientjes@google.com, Iurii Zaikin , jdike@addtoit.com, dan.carpenter@oracle.com, devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org, Tim.Bird@sony.com, linux-um@lists.infradead.org, rostedt@goodmis.org, julia.lawall@lip6.fr, jpoimboe@redhat.com, kunit-dev@googlegroups.com, tytso@mit.edu, richard@nod.at, gregkh@linuxfoundation.org, rdunlap@infradead.org, linux-kernel@vger.kernel.org, mcgrof@kernel.org, keescook@google.com, linux-fsdevel@vger.kernel.org, log List-Id: linux-nvdimm@lists.01.org T24gRnJpLCBKdW4gMDcsIDIwMTkgYXQgMTI6MDA6NDdQTSAtMDcwMCwgU3RlcGhlbiBCb3lkIHdy b3RlOgo+IFF1b3RpbmcgSXVyaWkgWmFpa2luICgyMDE5LTA2LTA1IDE4OjI5OjQyKQo+ID4gT24g RnJpLCBNYXkgMTcsIDIwMTkgYXQgMTE6MjIgQU0gU3RlcGhlbiBCb3lkIDxzYm95ZEBrZXJuZWwu b3JnPiB3cm90ZToKPiA+ID4KPiA+ID4gUXVvdGluZyBCcmVuZGFuIEhpZ2dpbnMgKDIwMTktMDUt MTQgMTU6MTc6MTApCj4gPiA+ID4gZGlmZiAtLWdpdCBhL2tlcm5lbC9zeXNjdGwtdGVzdC5jIGIv a2VybmVsL3N5c2N0bC10ZXN0LmMKPiA+ID4gPiBuZXcgZmlsZSBtb2RlIDEwMDY0NAo+ID4gPiA+ IGluZGV4IDAwMDAwMDAwMDAwMDAuLmZlMGYyYmFlNjYwODUKPiA+ID4gPiAtLS0gL2Rldi9udWxs Cj4gPiA+ID4gKysrIGIva2VybmVsL3N5c2N0bC10ZXN0LmMKPiA+ID4gPiArCj4gPiA+ID4gKwo+ ID4gPiA+ICtzdGF0aWMgdm9pZCBzeXNjdGxfdGVzdF9kb2ludHZlY19oYXBweV9zaW5nbGVfbmVn YXRpdmUoc3RydWN0IGt1bml0ICp0ZXN0KQo+ID4gPiA+ICt7Cj4gPiA+ID4gKyAgICAgICBzdHJ1 Y3QgY3RsX3RhYmxlIHRhYmxlID0gewo+ID4gPiA+ICsgICAgICAgICAgICAgICAucHJvY25hbWUg PSAiZm9vIiwKPiA+ID4gPiArICAgICAgICAgICAgICAgLmRhdGEgICAgICAgICAgID0gJnRlc3Rf ZGF0YS5pbnRfMDAwMSwKPiA+ID4gPiArICAgICAgICAgICAgICAgLm1heGxlbiAgICAgICAgID0g c2l6ZW9mKGludCksCj4gPiA+ID4gKyAgICAgICAgICAgICAgIC5tb2RlICAgICAgICAgICA9IDA2 NDQsCj4gPiA+ID4gKyAgICAgICAgICAgICAgIC5wcm9jX2hhbmRsZXIgICA9IHByb2NfZG9pbnR2 ZWMsCj4gPiA+ID4gKyAgICAgICAgICAgICAgIC5leHRyYTEgICAgICAgICA9ICZpX3plcm8sCj4g PiA+ID4gKyAgICAgICAgICAgICAgIC5leHRyYTIgICAgICAgICA9ICZpX29uZV9odW5kcmVkLAo+ ID4gPiA+ICsgICAgICAgfTsKPiA+ID4gPiArICAgICAgIGNoYXIgaW5wdXRbXSA9ICItOSI7Cj4g PiA+ID4gKyAgICAgICBzaXplX3QgbGVuID0gc2l6ZW9mKGlucHV0KSAtIDE7Cj4gPiA+ID4gKyAg ICAgICBsb2ZmX3QgcG9zID0gMDsKPiA+ID4gPiArCj4gPiA+ID4gKyAgICAgICB0YWJsZS5kYXRh ID0ga3VuaXRfa3phbGxvYyh0ZXN0LCBzaXplb2YoaW50KSwgR0ZQX1VTRVIpOwo+ID4gPiA+ICsg ICAgICAgS1VOSVRfRVhQRUNUX0VRKHRlc3QsIDAsIHByb2NfZG9pbnR2ZWMoJnRhYmxlLCAxLCBp bnB1dCwgJmxlbiwgJnBvcykpOwo+ID4gPiA+ICsgICAgICAgS1VOSVRfRVhQRUNUX0VRKHRlc3Qs IHNpemVvZihpbnB1dCkgLSAxLCBsZW4pOwo+ID4gPiA+ICsgICAgICAgS1VOSVRfRVhQRUNUX0VR KHRlc3QsIHNpemVvZihpbnB1dCkgLSAxLCBwb3MpOwo+ID4gPiA+ICsgICAgICAgS1VOSVRfRVhQ RUNUX0VRKHRlc3QsIC05LCAqKGludCAqKXRhYmxlLmRhdGEpOwo+ID4gPgo+ID4gPiBJcyB0aGUg Y2FzdGluZyBuZWNlc3Nhcnk/IE9yIGNhbiB0aGUgbWFjcm8gZG8gYSB0eXBlIGNvZXJjaW9uIG9m IHRoZQo+ID4gPiBzZWNvbmQgcGFyYW1ldGVyIGJhc2VkIG9uIHRoZSBmaXJzdCB0eXBlPwo+ID4g IERhdGEgZmllbGQgaXMgZGVmaW5lZCBhcyB2b2lkKiBzbyBJIGJlbGlldmUgY2FzdGluZyBpcyBu ZWNlc3NhcnkgdG8KPiA+IGRlcmVmZXJlbmNlIGl0IGFzIGEgcG9pbnRlciB0byBhbiBhcnJheSBv ZiBpbnRzLiBJIGRvbid0IHRoaW5rIHRoZQo+ID4gbWFjcm8gc2hvdWxkIGRvIGFueSB0eXBlIGNv ZXJjaW9uIHRoYXQgPT0gb3BlcmF0b3Igd291bGRuJ3QgZG8uCj4gPiAgSSBkaWQgY2hhbmdlIHRo ZSBjYXN0IHRvIG1ha2UgaXQgbW9yZSBjbGVhciB0aGF0IGl0J3MgYSBwb2ludGVyIHRvIGFuCj4g PiBhcnJheSBvZiBpbnRzIGJlaW5nIGRlcmVmZXJlbmNlZC4KPiAKPiBPaywgSSBzdGlsbCB3b25k ZXIgaWYgd2Ugc2hvdWxkIG1ha2UgS1VOSVRfRVhQRUNUX0VRIGNoZWNrIHRoZSB0eXBlcyBvbgo+ IGJvdGggc2lkZXMgYW5kIGNhdXNlIGEgYnVpbGQgd2FybmluZy9lcnJvciBpZiB0aGUgdHlwZXMg YXJlbid0IHRoZSBzYW1lLgo+IFRoaXMgd291bGQgYmUgc2ltaWxhciB0byBvdXIgbWluL21heCBt YWNyb3MgdGhhdCBjb21wbGFpbiBhYm91dAo+IG1pc21hdGNoZWQgdHlwZXMgaW4gdGhlIGNvbXBh cmlzb25zLiBUaGVuIGlmIGEgdGVzdCBkZXZlbG9wZXIgbmVlZHMgdG8KPiBjb252ZXJ0IG9uZSB0 eXBlIG9yIHRoZSBvdGhlciB0aGV5IGNvdWxkIGRvIHNvIHdpdGggYQo+IEtVTklUX0VYUEVDVF9F UV9UKCkgbWFjcm8gdGhhdCBsaXN0cyB0aGUgdHlwZXMgdG8gY29lcmNlIGJvdGggc2lkZXMgdG8K PiBleHBsaWNpdGx5LgoKRG8geW91IHRoaW5rIGl0IHdvdWxkIGJlIGJldHRlciB0byBkbyBhIHBo b255IGNvbXBhcmUgc2ltaWxhciB0byBob3cKbWluL21heCB1c2VkIHRvIHdvcmsgcHJpb3IgdG8g NC4xNywgb3IgdG8gdXNlIHRoZSBuZXcgX190eXBlY2hlY2soLi4uKQptYWNybz8gVGhpcyBtaWdo dCBzZWVtIGxpa2UgYSBkdW1iIHF1ZXN0aW9uIChhbmQgbWF5YmUgaXQgaXMpLCBidXQgSXVyaWkK YW5kIEkgdGhvdWdodCB0aGUgZm9ybWVyIGNyZWF0ZWQgYW4gZXJyb3IgbWVzc2FnZSB0aGF0IHdh cyBhIGJpdCBlYXNpZXIKdG8gdW5kZXJzdGFuZCwgd2hlcmVhcyBfX3R5cGVjaGVjayBpcyBvYnZp b3VzbHkgc3VwZXJpb3IgaW4gdGVybXMgb2YKY29kZSByZXVzZS4KClRoaXMgaXMgd2hhdCB3ZSBh cmUgdGhpbmtpbmcgcmlnaHQgbm93OyBpZiB5b3UgZG9uJ3QgaGF2ZSBhbnkgY29tcGxhaW50cwpJ IHdpbGwgc3F1YXNoIGl0IGludG8gdGhlIHJlbGV2YW50IGNvbW1pdHMgb24gdGhlIG5leHQgcmV2 aXNpb246Ci0tLQpGcm9tOiBJdXJpaSBaYWlraW4gPHl6YWlraW5AZ29vZ2xlLmNvbT4KCkFkZHMg YSB3YXJuaW5nIG1lc3NhZ2Ugd2hlbiBjb21wYXJpbmcgdmFsdWVzIG9mIGRpZmZlcmVudCB0eXBl cyBzaW1pbGFyCnRvIHdoYXQgbWluKCkgLyBtYXgoKSBtYWNyb3MgZG8uCgpTaWduZWQtb2ZmLWJ5 OiBJdXJpaSBaYWlraW4gPHl6YWlraW5AZ29vZ2xlLmNvbT4KLS0tCiBpbmNsdWRlL2t1bml0L3Rl c3QuaCB8IDE1ICsrKysrKysrKysrKysrKwogMSBmaWxlIGNoYW5nZWQsIDE1IGluc2VydGlvbnMo KykKCmRpZmYgLS1naXQgYS9pbmNsdWRlL2t1bml0L3Rlc3QuaCBiL2luY2x1ZGUva3VuaXQvdGVz dC5oCmluZGV4IDUxMWM5ZTg1NDAxYTYuLjc5MWUyMmZiYTU2MjAgMTAwNjQ0Ci0tLSBhL2luY2x1 ZGUva3VuaXQvdGVzdC5oCisrKyBiL2luY2x1ZGUva3VuaXQvdGVzdC5oCkBAIC0zMzUsNiArMzM1 LDEzIEBAIHZvaWQgX19wcmludGYoMywgNCkga3VuaXRfcHJpbnRrKGNvbnN0IGNoYXIgKmxldmVs LAogI2RlZmluZSBrdW5pdF9lcnIodGVzdCwgZm10LCAuLi4pIFwKIAkJa3VuaXRfcHJpbnRrKEtF Uk5fRVJSLCB0ZXN0LCBmbXQsICMjX19WQV9BUkdTX18pCiAKKy8qCisgKiAnVW5uZWNlc3Nhcnkn IGNhc3Qgc2VydmVzIHRvIGdlbmVyYXRlIGEgY29tcGlsZS10aW1lIHdhcm5pbmcgaW4gY2FzZQor ICogb2YgY29tcGFyaW5nIGluY29tcGF0aWJsZSB0eXBlcy4gSW5zcGlyZWQgYnkgaW5jbHVkZS9s aW51eC9rZXJuZWwuaAorICovCisjZGVmaW5lIF9fa3VuaXRfdHlwZWNoZWNrKGxocywgcmhzKSBc CisJKCh2b2lkKSAoJihsaHMpID09ICYocmhzKSkpCisKIHN0YXRpYyBpbmxpbmUgc3RydWN0IGt1 bml0X3N0cmVhbSAqa3VuaXRfZXhwZWN0X3N0YXJ0KHN0cnVjdCBrdW5pdCAqdGVzdCwKIAkJCQkJ CSAgICAgIGNvbnN0IGNoYXIgKmZpbGUsCiAJCQkJCQkgICAgICBjb25zdCBjaGFyICpsaW5lKQpA QCAtNTE0LDYgKzUyMSw3IEBAIHN0YXRpYyBpbmxpbmUgdm9pZCBrdW5pdF9leHBlY3RfcHRyX2Jp bmFyeShzdHJ1Y3Qga3VuaXQgKnRlc3QsCiAjZGVmaW5lIEtVTklUX0VYUEVDVF9CSU5BUlkodGVz dCwgbGVmdCwgY29uZGl0aW9uLCByaWdodCkgZG8gewkJICAgICAgIFwKIAl0eXBlb2YobGVmdCkg X19sZWZ0ID0gKGxlZnQpOwkJCQkJICAgICAgIFwKIAl0eXBlb2YocmlnaHQpIF9fcmlnaHQgPSAo cmlnaHQpOwkJCQkgICAgICAgXAorCV9fa3VuaXRfdHlwZWNoZWNrKF9fbGVmdCwgX19yaWdodCk7 CQkJCSAgICAgICBcCiAJa3VuaXRfZXhwZWN0X2JpbmFyeSh0ZXN0LAkJCQkJICAgICAgIFwKIAkJ CSAgICAobG9uZyBsb25nKSBfX2xlZnQsICNsZWZ0LAkJCSAgICAgICBcCiAJCQkgICAgKGxvbmcg bG9uZykgX19yaWdodCwgI3JpZ2h0LAkJICAgICAgIFwKQEAgLTUyNCw2ICs1MzIsNyBAQCBzdGF0 aWMgaW5saW5lIHZvaWQga3VuaXRfZXhwZWN0X3B0cl9iaW5hcnkoc3RydWN0IGt1bml0ICp0ZXN0 LAogI2RlZmluZSBLVU5JVF9FWFBFQ1RfQklOQVJZX01TRyh0ZXN0LCBsZWZ0LCBjb25kaXRpb24s IHJpZ2h0LCBmbXQsIC4uLikgZG8geyAgIFwKIAl0eXBlb2YobGVmdCkgX19sZWZ0ID0gKGxlZnQp OwkJCQkJICAgICAgIFwKIAl0eXBlb2YocmlnaHQpIF9fcmlnaHQgPSAocmlnaHQpOwkJCQkgICAg ICAgXAorCV9fa3VuaXRfdHlwZWNoZWNrKF9fbGVmdCwgX19yaWdodCk7CQkJCSAgICAgICBcCiAJ a3VuaXRfZXhwZWN0X2JpbmFyeV9tc2codGVzdCwJCQkJCSAgICAgICBcCiAJCQkJKGxvbmcgbG9u ZykgX19sZWZ0LCAjbGVmdCwJCSAgICAgICBcCiAJCQkJKGxvbmcgbG9uZykgX19yaWdodCwgI3Jp Z2h0LAkJICAgICAgIFwKQEAgLTUzOCw2ICs1NDcsNyBAQCBzdGF0aWMgaW5saW5lIHZvaWQga3Vu aXRfZXhwZWN0X3B0cl9iaW5hcnkoc3RydWN0IGt1bml0ICp0ZXN0LAogI2RlZmluZSBLVU5JVF9F WFBFQ1RfUFRSX0JJTkFSWSh0ZXN0LCBsZWZ0LCBjb25kaXRpb24sIHJpZ2h0KSBkbyB7CSAgICAg ICBcCiAJdHlwZW9mKGxlZnQpIF9fbGVmdCA9IChsZWZ0KTsJCQkJCSAgICAgICBcCiAJdHlwZW9m KHJpZ2h0KSBfX3JpZ2h0ID0gKHJpZ2h0KTsJCQkJICAgICAgIFwKKwlfX2t1bml0X3R5cGVjaGVj ayhfX2xlZnQsIF9fcmlnaHQpOwkJCQkgICAgICAgXAogCWt1bml0X2V4cGVjdF9wdHJfYmluYXJ5 KHRlc3QsCQkJCQkgICAgICAgXAogCQkJICAgICh2b2lkICopIF9fbGVmdCwgI2xlZnQsCQkJICAg ICAgIFwKIAkJCSAgICAodm9pZCAqKSBfX3JpZ2h0LCAjcmlnaHQsCQkJICAgICAgIFwKQEAgLTU1 Myw2ICs1NjMsNyBAQCBzdGF0aWMgaW5saW5lIHZvaWQga3VuaXRfZXhwZWN0X3B0cl9iaW5hcnko c3RydWN0IGt1bml0ICp0ZXN0LAogCQkJCSAgICAuLi4pIGRvIHsJCQkJICAgICAgIFwKIAl0eXBl b2YobGVmdCkgX19sZWZ0ID0gKGxlZnQpOwkJCQkJICAgICAgIFwKIAl0eXBlb2YocmlnaHQpIF9f cmlnaHQgPSAocmlnaHQpOwkJCQkgICAgICAgXAorCV9fa3VuaXRfdHlwZWNoZWNrKF9fbGVmdCwg X19yaWdodCk7CQkJCSAgICAgICBcCiAJa3VuaXRfZXhwZWN0X3B0cl9iaW5hcnlfbXNnKHRlc3Qs CQkJCSAgICAgICBcCiAJCQkJICAgICh2b2lkICopIF9fbGVmdCwgI2xlZnQsCQkgICAgICAgXAog CQkJCSAgICAodm9pZCAqKSBfX3JpZ2h0LCAjcmlnaHQsCQkgICAgICAgXApAQCAtMTAxMyw2ICsx MDI0LDcgQEAgc3RhdGljIGlubGluZSB2b2lkIGt1bml0X2Fzc2VydF9wdHJfYmluYXJ5KHN0cnVj dCBrdW5pdCAqdGVzdCwKICNkZWZpbmUgS1VOSVRfQVNTRVJUX0JJTkFSWSh0ZXN0LCBsZWZ0LCBj b25kaXRpb24sIHJpZ2h0KSBkbyB7CQkgICAgICAgXAogCXR5cGVvZihsZWZ0KSBfX2xlZnQgPSAo bGVmdCk7CQkJCQkgICAgICAgXAogCXR5cGVvZihyaWdodCkgX19yaWdodCA9IChyaWdodCk7CQkJ CSAgICAgICBcCisJX19rdW5pdF90eXBlY2hlY2soX19sZWZ0LCBfX3JpZ2h0KTsJCQkJICAgICAg IFwKIAlrdW5pdF9hc3NlcnRfYmluYXJ5KHRlc3QsCQkJCQkgICAgICAgXAogCQkJICAgIChsb25n IGxvbmcpIF9fbGVmdCwgI2xlZnQsCQkJICAgICAgIFwKIAkJCSAgICAobG9uZyBsb25nKSBfX3Jp Z2h0LCAjcmlnaHQsCQkgICAgICAgXApAQCAtMTAyMyw2ICsxMDM1LDcgQEAgc3RhdGljIGlubGlu ZSB2b2lkIGt1bml0X2Fzc2VydF9wdHJfYmluYXJ5KHN0cnVjdCBrdW5pdCAqdGVzdCwKICNkZWZp bmUgS1VOSVRfQVNTRVJUX0JJTkFSWV9NU0codGVzdCwgbGVmdCwgY29uZGl0aW9uLCByaWdodCwg Zm10LCAuLi4pIGRvIHsgICBcCiAJdHlwZW9mKGxlZnQpIF9fbGVmdCA9IChsZWZ0KTsJCQkJCSAg ICAgICBcCiAJdHlwZW9mKHJpZ2h0KSBfX3JpZ2h0ID0gKHJpZ2h0KTsJCQkJICAgICAgIFwKKwlf X2t1bml0X3R5cGVjaGVjayhfX2xlZnQsIF9fcmlnaHQpOwkJCQkgICAgICAgXAogCWt1bml0X2Fz c2VydF9iaW5hcnlfbXNnKHRlc3QsCQkJCQkgICAgICAgXAogCQkJCShsb25nIGxvbmcpIF9fbGVm dCwgI2xlZnQsCQkgICAgICAgXAogCQkJCShsb25nIGxvbmcpIF9fcmlnaHQsICNyaWdodCwJCSAg ICAgICBcCkBAIC0xMDM3LDYgKzEwNTAsNyBAQCBzdGF0aWMgaW5saW5lIHZvaWQga3VuaXRfYXNz ZXJ0X3B0cl9iaW5hcnkoc3RydWN0IGt1bml0ICp0ZXN0LAogI2RlZmluZSBLVU5JVF9BU1NFUlRf UFRSX0JJTkFSWSh0ZXN0LCBsZWZ0LCBjb25kaXRpb24sIHJpZ2h0KSBkbyB7CSAgICAgICBcCiAJ dHlwZW9mKGxlZnQpIF9fbGVmdCA9IChsZWZ0KTsJCQkJCSAgICAgICBcCiAJdHlwZW9mKHJpZ2h0 KSBfX3JpZ2h0ID0gKHJpZ2h0KTsJCQkJICAgICAgIFwKKwlfX2t1bml0X3R5cGVjaGVjayhfX2xl ZnQsIF9fcmlnaHQpOwkJCQkgICAgICAgXAogCWt1bml0X2Fzc2VydF9wdHJfYmluYXJ5KHRlc3Qs CQkJCQkgICAgICAgXAogCQkJCSh2b2lkICopIF9fbGVmdCwgI2xlZnQsCQkJICAgICAgIFwKIAkJ CQkodm9pZCAqKSBfX3JpZ2h0LCAjcmlnaHQsCQkgICAgICAgXApAQCAtMTA1MSw2ICsxMDY1LDcg QEAgc3RhdGljIGlubGluZSB2b2lkIGt1bml0X2Fzc2VydF9wdHJfYmluYXJ5KHN0cnVjdCBrdW5p dCAqdGVzdCwKIAkJCQkgICAgZm10LCAuLi4pIGRvIHsJCQkgICAgICAgXAogCXR5cGVvZihsZWZ0 KSBfX2xlZnQgPSAobGVmdCk7CQkJCQkgICAgICAgXAogCXR5cGVvZihyaWdodCkgX19yaWdodCA9 IChyaWdodCk7CQkJCSAgICAgICBcCisJX19rdW5pdF90eXBlY2hlY2soX19sZWZ0LCBfX3JpZ2h0 KTsJCQkJICAgICAgIFwKIAlrdW5pdF9hc3NlcnRfcHRyX2JpbmFyeV9tc2codGVzdCwJCQkJICAg ICAgIFwKIAkJCQkgICAgKHZvaWQgKikgX19sZWZ0LCAjbGVmdCwJCSAgICAgICBcCiAJCQkJICAg ICh2b2lkICopIF9fcmlnaHQsICNyaWdodCwJCSAgICAgICBcCi0tIAoyLjIyLjAucmMyLjM4My5n ZjRmYmJmMzBjMi1nb29nCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3Rv cC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmkt ZGV2ZWw= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pl1-x641.google.com ([2607:f8b0:4864:20::641]) by bombadil.infradead.org with esmtps (Exim 4.92 #3 (Red Hat Linux)) id 1hal2d-0006r1-TQ for linux-um@lists.infradead.org; Tue, 11 Jun 2019 17:58:42 +0000 Received: by mail-pl1-x641.google.com with SMTP id bi6so5040772plb.12 for ; Tue, 11 Jun 2019 10:58:37 -0700 (PDT) Date: Tue, 11 Jun 2019 10:58:30 -0700 From: Brendan Higgins Subject: Re: [PATCH v4 17/18] kernel/sysctl-test: Add null pointer test for sysctl.c:proc_dointvec() Message-ID: <20190611175830.GA236872@google.com> References: <20190514221711.248228-1-brendanhiggins@google.com> <20190514221711.248228-18-brendanhiggins@google.com> <20190517182254.548EA20815@mail.kernel.org> <20190607190047.C3E7A20868@mail.kernel.org> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20190607190047.C3E7A20868@mail.kernel.org> List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-um" Errors-To: linux-um-bounces+geert=linux-m68k.org@lists.infradead.org To: Stephen Boyd Cc: pmladek@suse.com, linux-doc@vger.kernel.org, peterz@infradead.org, amir73il@gmail.com, dri-devel@lists.freedesktop.org, Alexander.Levin@microsoft.com, yamada.masahiro@socionext.com, mpe@ellerman.id.au, linux-kselftest@vger.kernel.org, shuah@kernel.org, robh@kernel.org, linux-nvdimm@lists.01.org, frowand.list@gmail.com, knut.omang@oracle.com, kieran.bingham@ideasonboard.com, wfg@linux.intel.com, joel@jms.id.au, rientjes@google.com, Iurii Zaikin , jdike@addtoit.com, dan.carpenter@oracle.com, devicetree@vger.kernel.org, linux-kbuild@vger.kernel.org, Tim.Bird@sony.com, linux-um@lists.infradead.org, rostedt@goodmis.org, julia.lawall@lip6.fr, jpoimboe@redhat.com, kunit-dev@googlegroups.com, tytso@mit.edu, richard@nod.at, gregkh@linuxfoundation.org, rdunlap@infradead.org, linux-kernel@vger.kernel.org, mcgrof@kernel.org, daniel@ffwll.ch, keescook@google.com, linux-fsdevel@vger.kernel.org, logang@deltatee.com, khilman@baylibre.com On Fri, Jun 07, 2019 at 12:00:47PM -0700, Stephen Boyd wrote: > Quoting Iurii Zaikin (2019-06-05 18:29:42) > > On Fri, May 17, 2019 at 11:22 AM Stephen Boyd wrote: > > > > > > Quoting Brendan Higgins (2019-05-14 15:17:10) > > > > diff --git a/kernel/sysctl-test.c b/kernel/sysctl-test.c > > > > new file mode 100644 > > > > index 0000000000000..fe0f2bae66085 > > > > --- /dev/null > > > > +++ b/kernel/sysctl-test.c > > > > + > > > > + > > > > +static void sysctl_test_dointvec_happy_single_negative(struct kunit *test) > > > > +{ > > > > + struct ctl_table table = { > > > > + .procname = "foo", > > > > + .data = &test_data.int_0001, > > > > + .maxlen = sizeof(int), > > > > + .mode = 0644, > > > > + .proc_handler = proc_dointvec, > > > > + .extra1 = &i_zero, > > > > + .extra2 = &i_one_hundred, > > > > + }; > > > > + char input[] = "-9"; > > > > + size_t len = sizeof(input) - 1; > > > > + loff_t pos = 0; > > > > + > > > > + table.data = kunit_kzalloc(test, sizeof(int), GFP_USER); > > > > + KUNIT_EXPECT_EQ(test, 0, proc_dointvec(&table, 1, input, &len, &pos)); > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, len); > > > > + KUNIT_EXPECT_EQ(test, sizeof(input) - 1, pos); > > > > + KUNIT_EXPECT_EQ(test, -9, *(int *)table.data); > > > > > > Is the casting necessary? Or can the macro do a type coercion of the > > > second parameter based on the first type? > > Data field is defined as void* so I believe casting is necessary to > > dereference it as a pointer to an array of ints. I don't think the > > macro should do any type coercion that == operator wouldn't do. > > I did change the cast to make it more clear that it's a pointer to an > > array of ints being dereferenced. > > Ok, I still wonder if we should make KUNIT_EXPECT_EQ check the types on > both sides and cause a build warning/error if the types aren't the same. > This would be similar to our min/max macros that complain about > mismatched types in the comparisons. Then if a test developer needs to > convert one type or the other they could do so with a > KUNIT_EXPECT_EQ_T() macro that lists the types to coerce both sides to > explicitly. Do you think it would be better to do a phony compare similar to how min/max used to work prior to 4.17, or to use the new __typecheck(...) macro? This might seem like a dumb question (and maybe it is), but Iurii and I thought the former created an error message that was a bit easier to understand, whereas __typecheck is obviously superior in terms of code reuse. This is what we are thinking right now; if you don't have any complaints I will squash it into the relevant commits on the next revision: --- From: Iurii Zaikin Adds a warning message when comparing values of different types similar to what min() / max() macros do. Signed-off-by: Iurii Zaikin --- include/kunit/test.h | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/include/kunit/test.h b/include/kunit/test.h index 511c9e85401a6..791e22fba5620 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -335,6 +335,13 @@ void __printf(3, 4) kunit_printk(const char *level, #define kunit_err(test, fmt, ...) \ kunit_printk(KERN_ERR, test, fmt, ##__VA_ARGS__) +/* + * 'Unnecessary' cast serves to generate a compile-time warning in case + * of comparing incompatible types. Inspired by include/linux/kernel.h + */ +#define __kunit_typecheck(lhs, rhs) \ + ((void) (&(lhs) == &(rhs))) + static inline struct kunit_stream *kunit_expect_start(struct kunit *test, const char *file, const char *line) @@ -514,6 +521,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_binary(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -524,6 +532,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_binary_msg(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -538,6 +547,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, #define KUNIT_EXPECT_PTR_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_ptr_binary(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -553,6 +563,7 @@ static inline void kunit_expect_ptr_binary(struct kunit *test, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_expect_ptr_binary_msg(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -1013,6 +1024,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_binary(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -1023,6 +1035,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_BINARY_MSG(test, left, condition, right, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_binary_msg(test, \ (long long) __left, #left, \ (long long) __right, #right, \ @@ -1037,6 +1050,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, #define KUNIT_ASSERT_PTR_BINARY(test, left, condition, right) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_ptr_binary(test, \ (void *) __left, #left, \ (void *) __right, #right, \ @@ -1051,6 +1065,7 @@ static inline void kunit_assert_ptr_binary(struct kunit *test, fmt, ...) do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + __kunit_typecheck(__left, __right); \ kunit_assert_ptr_binary_msg(test, \ (void *) __left, #left, \ (void *) __right, #right, \ -- 2.22.0.rc2.383.gf4fbbf30c2-goog _______________________________________________ linux-um mailing list linux-um@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-um