Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix RPMFusion builds for armv7hl/ppc64le/aarch64 #3223

Merged
merged 4 commits into from Oct 27, 2020
Merged

Fix RPMFusion builds for armv7hl/ppc64le/aarch64 #3223

merged 4 commits into from Oct 27, 2020

Conversation

uklotzde
Copy link
Contributor

@uklotzde uklotzde commented Oct 26, 2020 •
edited

Please consult the build.log files of all failed builds:
http://koji.rpmfusion.org/koji/taskinfo?taskID=443529

@uklotzde uklotzde added the build label Oct 26, 2020
@uklotzde uklotzde added this to the 2.3.0 milestone Oct 26, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Oct 26, 2020

LGTM, waiting for CI.

@uklotzde uklotzde changed the title BalanceGroupState: Fix signed/unsigned comparison for armv7hl Fix RPMFusion builds for armv7hl/ppc64le/aarch64 Oct 26, 2020
@uklotzde
Copy link
Contributor Author

I have started a new build with non-fatal warnings to detect any further issues (instead of aborting the build after the first warning):
http://koji.rpmfusion.org/koji/taskinfo?taskID=443567

@uklotzde
Copy link
Contributor Author

@be Ready. I have hopefully fixed all warnings that appeared in the build logs.

daschuer
daschuer requested changes Oct 27, 2020
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you.

Does the arm compiler use unsigned char?
That might be dangerous because, because I am sure that our code base is not fully tested with that.

src/effects/builtin/metronomeeffect.cpp Show resolved Hide resolved
src/effects/builtin/metronomeeffect.cpp Outdated Show resolved Hide resolved
@uklotzde
Copy link
Contributor Author

uklotzde commented Oct 27, 2020 •
edited

Thank you.

Does the arm compiler use unsigned char?
That might be dangerous because, because I am sure that our code base is not fully tested with that.

Yes, char seems to be unsigned for that architecture/compiler and must be explicitly cast to a signed char when needed. Only the FAAD2 function signature is affected, casting is safe. Not further issues.

Correct code must not make any assumptions about the signedness of char, neither signed nor unsigned.

daschuer
daschuer reviewed Oct 27, 2020
View reviewed changes
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you.

@daschuer daschuer merged commit 4ab6017 into mixxxdj:2.3 Oct 27, 2020
@uklotzde uklotzde deleted the balanceffect branch November 1, 2020 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daschuer daschuer daschuer approved these changes

Assignees
No one assigned
Labels
build
Projects
None yet
Milestone
2.3.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@uklotzde @Be-ing @daschuer