[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v6 1/4] xen: introduce SYMBOL

On Thu, 10 Jan 2019, 12:29 Stefano Stabellini, <sstabellini@xxxxxxxxxx> wrote:
On Thu, 10 Jan 2019, Jan Beulich wrote:
> >>> On 10.01.19 at 03:40, <julien.grall@xxxxxxxxx> wrote:
> > On Wed, 9 Jan 2019, 18:43 Stefano Stabellini, <sstabellini@xxxxxxxxxx>
> > wrote:
> >
> >> Introduce a macro, SYMBOL, which is similar to RELOC_HIDE, but it is
> >> meant to be used everywhere symbols such as _stext and _etext are used
> >> in the code. It can take an array type as a parameter, and it returns
> >> the same type.
> >>
> >> SYMBOL is needed when accessing symbols such as _stext and _etext
> >> because the C standard forbids for both comparisons and substraction
> >> (see C Standard, 6.5.6 [ISO/IEC 9899:2011] and [1]) between pointers
> >> pointing to different objects. _stext, _etext, etc. are all pointers to
> >> different objects from ANCI C point of view.
> >>
> >
> > This does not make sense because you still return a pointer and therefore
> > the undefined behavior is still present.
> >
> > I really don't believe this patch is going to make the MISRA tool happy.
> Well, till now I've been assuming that no version of this series was
> posted without being certain the changes achieve the goal (of
> making that tool happy).

No, Jan: unfortunately we cannot re-run the scanning tool against any
version of Xen we wish to :-(

We cannot know in advance if a set of changes will make the tool happy
or not.

If I knew that SYMBOL returning the native point type as in v6 is
sufficient to make the tool happy, I wouldn't be here arguing. We cannot
know for sure until we commit the changes, then we ask PRQA to re-scan
against a more recent version of Xen. It is an heavy process and for
this reason I preferred the safer of the two approaches.

Anyway, I would rather get something in, even if insufficient, than
nothing. So I'll address all your comments based on returning the
pointer type, and submit a new version. The bothersome changes are the
ones to the call sites, and I would like to get those in no matter the
implementation of SYMBOL.

It is not only insufficient but wrong when you read the commit message. You also were not convinced about this approach. 

I understand that we need to commit so we can get the result from the PRQA tool. However, we should have talked with people knowing MISRA to understand whether it could work.

You also didn't address my point on why Linux needs to go through unsigned long.

So I don't think it is right to merge it without more ground.

On that basis:

Nacked-by: Julien Grall <julien.grall@xxxxxxx>

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.