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.

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

Improve BSD compatibility #1342

Merged
merged 5 commits into from
Nov 16, 2023
Merged

Improve BSD compatibility #1342

merged 5 commits into from
Nov 16, 2023

Conversation

uweseimet
Copy link
Contributor

@uweseimet uweseimet commented Nov 12, 2023

Tested with FreeBSD 14.0 and NetBSD 10.0, also see BSD setup instructions on https://github.com/PiSCSI/piscsi/wiki/Setup-Instructions.

Copy link

sonarcloud bot commented Nov 12, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@uweseimet uweseimet marked this pull request as ready for review November 12, 2023 11:11
@rdmark
Copy link
Member

rdmark commented Nov 15, 2023

@uweseimet i can’t see BSD setup instructions on the linked wiki page. Are they in fact on a different page? (BTW I added back notes about AIBOM and Gamernium board types to that page. I don’t know if you removed them by mistake?)

can you please remind be about the purpose of BSD support? Is this about running a BSD OS with GPIO on RPi hardware?

@uweseimet
Copy link
Contributor Author

uweseimet commented Nov 15, 2023

@uweseimet i can’t see BSD setup instructions on the linked wiki page. Are they in fact on a different page? (BTW I added back notes about AIBOM and Gamernium board types to that page. I don’t know if you removed them by mistake?)

These instructions have been commented out for ages, haven't they? But I might have missed something there. The NetBSD instructions are on https://github.com/PiSCSI/piscsi/wiki/Compilation-Instructions.

can you please remind be about the purpose of BSD support? Is this about running a BSD OS with GPIO on RPi hardware?

The purpose is to be able to compile and test piscsi also on BSD and macos. This is possible now, see the testing_macos_compile branch. In addition, using other platforms for compiling the sources ensures portability. In addition, you can run scsictl on other platforms and connect remotely to piscsi. I sometimes do this with my PC.
Note that as far as running piscsi on a Pi with NetBSD is concerned, I have no idea whether this works. There is no NetBSD for the Pi 4, by the way, only for older models. Even if it works, I would not recommend to support it. This project does not have the resources to test and support more platforms. Or to build release images for more platforms ;-).

My guess is that to run at least scsictl on a Windows PC (with the required libraries like protobuf installed) might be possible in the future. Or to run the unit tests on Windows.

One of the changes (the missing cast) was actually a minor bug.

Comment on lines +45 to +46
if (bind(service_socket, reinterpret_cast<const sockaddr*>(&server), //NOSONAR bit_cast is not supported by the bullseye compiler
static_cast<socklen_t>(sizeof(sockaddr_in))) < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Nitpick, but I guess you used 4 spaces for indentation here instead of the tabs that the rest of this file uses?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, is this because a new stylistic rule about using spaces instead of tabs for indentation?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's because of the Eclipse formatter. Spaces is the target format, but I don't yet want to reformat everything.
I suggest to have github ignore whitespaces, which can be configured when reviewing diffs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that's related to the C++ style ticket.

@uweseimet uweseimet merged commit bb60204 into develop Nov 16, 2023
16 checks passed
@uweseimet uweseimet deleted the bsd_compatibility branch November 16, 2023 11:37
rdmark pushed a commit that referenced this pull request May 1, 2024
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.

2 participants