From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] lib/igt_core: document the caveats of magic code blocks Date: Fri, 14 Mar 2014 16:29:37 +0100 Message-ID: <20140314152937.GV30571@phenom.ffwll.local> References: <1394781488-20586-1-git-send-email-daniel.vetter@ffwll.ch> <532316EC.3070400@linux.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f43.google.com (mail-ee0-f43.google.com [74.125.83.43]) by gabe.freedesktop.org (Postfix) with ESMTP id 1AA50FB4A8 for ; Fri, 14 Mar 2014 08:29:42 -0700 (PDT) Received: by mail-ee0-f43.google.com with SMTP id e53so1525233eek.30 for ; Fri, 14 Mar 2014 08:29:42 -0700 (PDT) Content-Disposition: inline In-Reply-To: <532316EC.3070400@linux.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces@lists.freedesktop.org Errors-To: intel-gfx-bounces@lists.freedesktop.org To: Tvrtko Ursulin Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Fri, Mar 14, 2014 at 02:49:16PM +0000, Tvrtko Ursulin wrote: > > On 03/14/2014 07:18 AM, Daniel Vetter wrote: > [snip] > >+ * # Magic Control Blocks > >+ * > >+ * i-g-t makes heavy use of C macros which serve as magic control blocks. They > >+ * work fairly well and transparently but since C doesn't have full-blown > >+ * closures there are caveats: > >+ * > >+ * - Asynchronous blocks which are used to spawn children internally use fork(). > >+ * Which means that impossible control flow like jumping out of the control > >+ * block is possible, but it will horribly badly the i-g-t library code. And > > I suggest to add a verb and reorder the "horribly badly" part of the > sentence a bit. Or perhaps you tried to illustrate the program flow > after jumping out of the control block? :) I did frob the paragraph a bit before pushing. Can you please take a look at what's committed and if you see some room for improvement send out another patch? > > >+ * - Code blocks with magic control flow are implemented with setjmp() and > >+ * longjmp(). This applies to #igt_fixture and #igt_subtest blocks and all the > >+ * three variants to finish test: igt_success(), igt_skip() and igt_fail(). > >+ * Mostly this is of no concern, except when such a control block changes > >+ * stack variables defined in the same function as the control block resides. > >+ * Any store/load behaviour after a longjmp() is ill-defined for these > >+ * variables. Avoid such code. > >+ * > >+ * Quoting the man page for longjmp(): > >+ * > >+ * "The values of automatic variables are unspecified after a call to > >+ * longjmp() if they meet all the following criteria:" > >+ * - "they are local to the function that made the corresponding setjmp() call; > >+ * - "their values are changed between the calls to setjmp() and longjmp(); and > >+ * - "they are not declared as volatile." > > */ > > So all variables which are getting initialised in igt_fixture should > be moved out to file scope? Gcc does warn about potential clobbering > although I haven't exactly gotten my head round understanding if and > when it can really happen with igt_fixture. Actually igt_fixture and igt_subtest, but usually it's igt_fixtures where something gets set up and then clobbered. What I usually do is push them out to global scope, but not at the top of the file but only right above igt_main. That tends to avoid gcc warnings about shadowing. Woud it be useful to add this bkm to the docs, too? Thanks for the feedback, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch