Re: [PATCH] Porting small OpenBSD changes.

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: David CARLIER <devnexen(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Porting small OpenBSD changes.
Date: 2017-11-20 22:24:03
Message-ID: 13084.1511216643@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

David CARLIER <devnexen(at)gmail(dot)com> writes:
> On 20 November 2017 at 18:49, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> OTOH, we still have M68K
>> and VAX stanzas in that file, so I suppose it's silly to complain
>> about 88K. A bigger issue is that I wonder whether that code has
>> ever been tested: it does not look to me like the __asm__ call is
>> even syntactically correct. There should be colons in it.

> True :-) corrected. Thanks.

I still dare to doubt whether you've tested this, because AFAICS
the operand numbering is wrong. The "r"(lock) operand is number 3
given these operand declarations, not number 2.

Our practice elsewhere in s_lock.h is to use a "+" constraint instead of
duplicated operands, and I think that's better style because it avoids any
question of whether you're supposed to count duplicated operands.

Also, per the comment near s_lock.h line 115, it's important to specify
"+m"(*lock) as an output operand so that gcc knows that the asm
clobbers *lock. It's possible that "memory" makes that redundant,
but I'd just as soon maintain consistency with the well-tested
other parts of the file.

So I propose the attached patch instead. It would still be a good idea
to actually test this ;-)

regards, tom lane

Attachment Content-Type Size
absorb-openbsd-fixes-v3.patch text/x-diff 1.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-11-20 22:36:15 Re: [PATCH] Porting small OpenBSD changes.
Previous Message Masahiko Sawada 2017-11-20 22:19:30 Re: [HACKERS] Moving relation extension locks out of heavyweight lock manager