All of lore.kernel.org
 help / color / mirror / Atom feed
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
To: Jan Beulich <JBeulich@suse.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>,
	Daniel De Graaf <dgdegra@tycho.nsa.gov>,
	Doug Goldstein <cardoe@cardoe.com>,
	xen-devel@lists.xenproject.org
Subject: Re: [PATCH v3] xsm: add a default policy to .init.data
Date: Tue, 5 Jul 2016 11:37:17 -0400	[thread overview]
Message-ID: <20160705153717.GE6428@char.us.oracle.com> (raw)
In-Reply-To: <577635B702000078000FA47E@prv-mh.provo.novell.com>

On Fri, Jul 01, 2016 at 01:19:51AM -0600, Jan Beulich wrote:
> >>> On 30.06.16 at 17:13, <konrad.wilk@oracle.com> wrote:
> > On Thu, Jun 30, 2016 at 10:01:18AM -0400, Daniel De Graaf wrote:
> >> On 06/30/2016 09:45 AM, Konrad Rzeszutek Wilk wrote:
> >> > On Wed, Jun 29, 2016 at 11:09:01AM -0400, Daniel De Graaf wrote:
> >> > > --- a/xen/xsm/xsm_core.c
> >> > > +++ b/xen/xsm/xsm_core.c
> >> > > @@ -36,6 +36,24 @@ static inline int verify(struct xsm_operations *ops)
> >> > >      return 0;
> >> > >  }
> >> > > 
> >> > > +#ifdef CONFIG_XSM_POLICY
> >> > > +extern char xsm_init_policy[];
> >> > 
> >> > > +extern int xsm_init_policy_size;
> >> > > +#else
> >> > > +#define xsm_init_policy 0
> >> > > +#endif
> >> > > +
> >> > > +static void __init xsm_policy_init(void)
> >> > > +{
> >> > > +#ifdef CONFIG_XSM_POLICY
> >> > > +    if ( policy_size == 0 )
> >> > > +    {
> >> > > +        policy_buffer = xsm_init_policy;
> >> > > +        policy_size = xsm_init_policy_size;
> >> > > +    }
> >> > > +#endif
> >> > > +}
> >> > > +
> >> > 
> >> > This all looks like it could go in a header file?
> >> 
> >> Sure, I could move it to xsm.h.  I thought it might be clearer to have the
> >> assignment and the xfree() check in the same file.
> > 
> > You are the maintainer of that code, so it is your call.
> > 
> > Having externs in C code is not that great (from my perspective),
> > so if you move all of this to a header file that should get easier?
> 
> I agree, fwiw, for the extern-s to be moved to a header. That
> header should then also be included by the generated source
> file producing the symbols. I don't, however, see why the
> function should go to a header.

I was thinking that since the function is a nop under #CONFIG_XSM_POLICY
you could just make it an inline static function, like:

#ifdef CONFIG_XSM_POLICY
static inline void  xsm_policy_init(void)
{...
}
#else
#define xsm_policy_init(void) ;
#endif

Or such.


But this is really minor, and pls ignore it if means doing more work than
neccessary.

> 
> Jan
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

      reply	other threads:[~2016-07-05 15:37 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-29 15:09 [PATCH v3] xsm: add a default policy to .init.data Daniel De Graaf
2016-06-30 13:45 ` Konrad Rzeszutek Wilk
2016-06-30 14:01   ` Daniel De Graaf
2016-06-30 15:13     ` Konrad Rzeszutek Wilk
2016-07-01  7:19       ` Jan Beulich
2016-07-05 15:37         ` Konrad Rzeszutek Wilk [this message]

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=20160705153717.GE6428@char.us.oracle.com \
    --to=konrad.wilk@oracle.com \
    --cc=JBeulich@suse.com \
    --cc=cardoe@cardoe.com \
    --cc=dgdegra@tycho.nsa.gov \
    --cc=ian.jackson@eu.citrix.com \
    --cc=xen-devel@lists.xenproject.org \
    /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.