Skip to content

Conversation

glaubitz
Copy link
Contributor

Referencing #40, here's the same PR for the master branch.

Thanks,
Adrian

glaubitz and others added 2 commits July 13, 2016 11:40
…ALIGN

On m68k, 'long long' is 16-bit aligned while 'sem_t' is 32-bit aligned
and we must therefore include 'sem_t' when determining the values for
FB_ALIGNMENT and FB_DOUBLE_ALIGN. Otherwise, the futex system call
will fail on these systems.
@AlexPeshkoff
Copy link
Member

Sorry, but you did not build firebird with your patch (or at least did not check how is resulting database marked)

@glaubitz
Copy link
Contributor Author

Hello Alex!

I'm not sure whether I can follow. What do you mean with "how the database is marked"? I'm willing to improve my code, so there is no need to close the PR. You don't need to close a PR and reopen a new PR for every change, one can just force-push changes, that's how systemd upstream does it.

Anyway, if you can provide a little more detailed explanation, I'm happy to make the necessary changes.

Thanks,
Adrian

@AlexPeshkoff
Copy link
Member

On 07/13/2016 02:20 PM, John Paul Adrian Glaubitz wrote:

Hello Alex!

I'm not sure whether I can follow. What do you mean with "how the database is marked"? I'm willing to improve my code, so there is no need to close the PR. You don't need to close a PR and reopen a new PR for every change, one can just force-push changes, that's how systemd upstream does it.

Anyway, if you can provide a little more detailed explanation, I'm happy to make the necessary changes.

./gstat -h
should return correct implementation line. In amd64 I get:

     Implementation          HW=AMD/Intel/x64 little-endian OS=Linux 

CC=gcc

In your patch you define CPU to something like CpuM68k, but there is no
such symbol at all (and no textual description for it). Therefore I
suppose that your #ifdef does not work at all (and you get some other HW
identifier active), or firebird will not compile at all - you will get
syntax error with CpuM68k.

Ports make sense when they are at least minimally tested on target HW.

@glaubitz
Copy link
Contributor Author

Hi Alex!

Thanks for the additional hints and very sorry for the late reply, I was very busy with work!

You're absolutely right, my patch is incomplete. I'm working on an updated version now and will update the PR once I'm ready. We can also drop the patch for Firebird2.5 as Debian just switched to Firebird3.0.

Thanks,
Adrian

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants