From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40140) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1dMsMh-0006Wk-Hc for qemu-devel@nongnu.org; Mon, 19 Jun 2017 04:48:56 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1dMsMd-0006e8-Lu for qemu-devel@nongnu.org; Mon, 19 Jun 2017 04:48:55 -0400 Received: from mx1.redhat.com ([209.132.183.28]:42872) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1dMsMd-0006cg-CW for qemu-devel@nongnu.org; Mon, 19 Jun 2017 04:48:51 -0400 Date: Mon, 19 Jun 2017 09:48:44 +0100 From: "Dr. David Alan Gilbert" Message-ID: <20170619084843.GB2104@work-vm> References: <20170613165313.20954-1-ehabkost@redhat.com> <20170613165313.20954-16-ehabkost@redhat.com> <20170615121407.GA2399@work-vm> <20170617193349.GM22043@thinpad.lan.raisama.net> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170617193349.GM22043@thinpad.lan.raisama.net> Subject: Re: [Qemu-devel] [RFC 15/15] [test only] Use 'Error *err[static 1]' instead of 'Error **errp' to catch NULL errp arguments List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Eduardo Habkost Cc: qemu-devel@nongnu.org, Markus Armbruster , Michael Roth * Eduardo Habkost (ehabkost@redhat.com) wrote: > On Thu, Jun 15, 2017 at 01:14:15PM +0100, Dr. David Alan Gilbert wrote: > > * Eduardo Habkost (ehabkost@redhat.com) wrote: > > > This is being included in the RFC just to help validating the series, > > > and ensure we don't use NULL errp anywhere. > > > > > > I don't propose we actually change all code to use the > > > 'Error *errp[static 1]' syntax because it confuses Cocinelle. > > > > > > I am considering including a assert(errp) line in the ERR_IS_SET() > > > macro, so we can catch NULL errp at runtime in case somebody forgets > > > about it. > > > > > > Generated by the following Coccinelle patch: > > > > > > @@ > > > typedef Error; > > > type T; > > > identifier FN !~ "os_mem_prealloc|qemu_fork"; > > > identifier errp; > > > @@ > > > T FN(..., > > > - Error **errp > > > + Error *errp[STATIC_1] > > > ); > > > > > > @@ > > > typedef Error; > > > type T; > > > identifier FN !~ "os_mem_prealloc|qemu_fork"; > > > identifier errp; > > > @@ > > > T FN(..., > > > - Error **errp > > > + Error *errp[STATIC_1] > > > ) { ... } > > > > > > Followed by the following sed command: > > > > > > $ sed -i -e 's/\[STATIC_1\]/[static 1]/g' $(g grep -w -l STATIC_1) > > > > > > Signed-off-by: Eduardo Habkost > > > --- > > > > > > > > > void visit_start_struct(Visitor *v, const char *name, void **obj, > > > - size_t size, Error **errp); > > > + size_t size, Error *errp[static 1]); > > > > > > > Is it possible to typedef that to something that hides the magic syntax? > > Then we could get that down to QError errp or something like that? > > Unfortunately it is not possible to hide it in a typedef: the > "static" keyword inside the square brackets is a feature of > function parameter declarations only. Using it on a typedef > results in: > > a.c:3:16: error: static or type qualifiers in non-parameter array declarator > typedef Error *ErrorPtr[static 1]; That's a shame; it's rather odd looking; still I think it's worth it if it simplifies the error code. > We could use a preprocessor macro, but I suspect this would also > confuse Coccinelle. > > -- > Eduardo Dave -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK