All of lore.kernel.org
 help / color / mirror / Atom feed
From: Cyril Hrubis <chrubis@suse.cz>
To: ltp@lists.linux.it
Subject: [LTP] [PATCH 1/2] SAFE_MACROS: Redirect to tst_brk_() early
Date: Thu, 9 Feb 2017 17:11:20 +0100	[thread overview]
Message-ID: <20170209161120.GA23109@rei> (raw)
In-Reply-To: <1235532957.2260942.1486655806975.JavaMail.zimbra@redhat.com>

Hi!
> > +#ifndef BRKM_REDIRECT_H__
> > +#define BRKM_REDIRECT_H__
> > +
> > +#include "ltp_priv.h"
> > +
> > +#ifdef tst_brkm
> > +# undef tst_brkm
> > +#endif
> > +
> > +#define tst_brkm(flags, cleanup, fmt, ...) do { \
> > +	if (tst_test) \
> > +		tst_brk_(__FILE__, __LINE__, flags, fmt, ##__VA_ARGS__); \
> > +	else \
> > +                tst_brkm_(__FILE__, __LINE__, flags, cleanup, fmt,
> > ##__VA_ARGS__); \
> > +	} while (0)
> 
> Stil trying to wrap my head around all this, but after first look,..
> How does this related to "tst_brkm_" from lib/tst_res.c?
> Can it be simplified? It doesn't look like we can reach newlib branch
> now, because tst_test condition is now in tst_brkm macro.

Well there more library code shared between oldlib and newlib than
SAFE_MACROS. So we can still reach tst_brkm_() from newlib, for instance
tst_device, tst_brkm, ... Hence the redirection has to stay.

Only change after this patch is that it cannot be reached from
SAFE_MACROS().

> Did we loose the check for "Non-NULL cleanup in newlib"?

Not really, since there is no way SAFE_MACRO would pass non-NULL cleanup
callback, since that is hardcoded in the tst_safe_macros.h header.

> > +#define tst_brkr(rval, flags, cleanup, fmt, ...) do { \
> > +		tst_brkm(flags, cleanup, fmt, ##__VA_ARGS__); \
> > +		return rval; \
> > +	} while (0)
> > +
> > +#define tst_brkv(flags, cleanup, fmt, ...) do { \
> > +		tst_brkm(flags, cleanup, fmt, ##__VA_ARGS__); \
> > +		return; \
> > +	} while (0)
> 
> All variants of tst_brk_ are more and more tricky to remember.
> Would it be too much to ask people "when writing new safe function,
> take into account that tst_brkm can return" and leave it up to
> them how they do it. Maybe they'll see goto more fitting.

I wanted to keep the changes minimal, but yes, we can as well
do explicit return instead of tst_brkr() and tst_brkv().

-- 
Cyril Hrubis
chrubis@suse.cz

  reply	other threads:[~2017-02-09 16:11 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-09 14:44 [LTP] [PATCH 1/2] SAFE_MACROS: Redirect to tst_brk_() early Cyril Hrubis
2017-02-09 14:44 ` [LTP] [PATCH 2/2] newlib: Allow SAFE_MACROS to be called from cleanup Cyril Hrubis
2017-02-09 15:56 ` [LTP] [PATCH 1/2] SAFE_MACROS: Redirect to tst_brk_() early Jan Stancek
2017-02-09 16:11   ` Cyril Hrubis [this message]
2017-02-10  8:27     ` Jan Stancek
2017-02-10  8:35       ` Jan Stancek
2017-02-10  9:38         ` Jan Stancek
2017-02-13  8:09           ` Cyril Hrubis
2017-02-13 13:30             ` Jan Stancek
2017-02-13 15:14               ` Cyril Hrubis

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170209161120.GA23109@rei \
    --to=chrubis@suse.cz \
    --cc=ltp@lists.linux.it \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.