Re: [PATCH RFC v2] m68k: remove get_fs()/set_fs()

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

 



On Wed, Jul 7, 2021 at 7:51 PM Linus Torvalds
<torvalds@xxxxxxxxxxxxxxxxxxxx> wrote:

Ok, strange. If this works for you, then the _concept_ is fine, and
there's something odd going on with the "macros with 'moves' vs 'move'
as a parameter" thing.

Oh, I think I see the problem.

Or rather, I see it in Christoph's version of the patch, I don't think
I've seen Michael's version, but that one was apparently very similar,
so maybe it has the same bug.

Christoph does:

   #define __put_user_asm(inst, res, x, ptr, bwl, reg)    \
   asm volatile ("\n"                                     \
          "1:     #inst."#bwl"    %2,%1\n"
   ...

and then uses it with code like

                __put_user_asm(MOVES, __pu_err, __pu_val, ptr, b, d);

and

                __get_user_asm("move", __gk_err, __gk_dst, __gk_src, u8, b, d);

and the issue is that there's a '#' too many.

The '#' turns the argument into a string, but it was already
_supposed_ to be a string. But no, the problem is that it turns the
macro name MOVES into the _string_ "MOVES".

Which happens to compile just fine, because "moves" is a real
instruction. But it's actually _meant_ to be a macro that expands to
either the string "moves" or "move".

So what happens is that at least in Christoph's version, I think the
code _always_ uses "MOVES", even in configurations where the macro
MOVES should have just become "move".

So it all builds fine, looks fine to the assembler, but it uses the
wrong instruction.

Macro expansion with the '#' character and other macros used as
arguments is something people need to be very careful with.  It's why
we have a whole header file for just the "stringify" operation, see
<linux/stringify.h>

But in this case, it shouldn't have used '#' at all, since the
argument was already a string, and never needed to be turned into a
string by the pre-processor.

             Linus



[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux