macOS port

[U2654]

adapted files for running under macOS

[U2654]

Actually, are you planning to merge the macOS port? Do you have any questions?

[nmeum]

Actually, are you planning to merge the macOS port?

I am actually not sure. We don't use Mac OS X and therefore do not have the means to test this or to ensure that it continues to work with ongoing development of the VP. The changes you are proposing also have the drawback that they complicate the existing code quite a bit by adding inline #ifdefs to a bunch of files, thereby making future modifications of this code more difficult. Instead of using #ifdef everywhere, a better approach would be to encapsulate non-portable code in OSX-specific files. For instance, you could add a new ./vp/src/compat directory in which you could add subdirectories for platform-depended code (e.g. ./vp/src/compat/osx). This directory could be added to the include path only if Mac OS X is used from vp/CMakeLists.txt. As such, files in this directory can be used to provide definitions not available on Mac OS X. Consider the non-portable byteswap.h header, instead of adding an #ifdef to every file including byteswap.h simply create ./vp/src/compat/osx/byteswap.h with the definitions used by the VP. A similar approach should also work for other changes you made. I cannot guarantee you that we will merge OS X support if this approach is adopted, but the goal should generally be to keep changes to existing files as minimal as possible. If doing so is not possible, I don't really see us merging this.

[U2654]

Actually, the macOS changes derive from BSD to Linux differences. For some changes, I could follow your approach. Some changes should be independent useful, e.g. 052ad74 and 414de56, and are not macOS specific.

However, there are some difficulties, e.g. with named and unnamed semaphores, to follow your suggestions. For this changes, I guess it could work to adapt code compatible with both platforms. I did changes like replacing ethhdr to ethhdrin 43b8b79. Such changes work on both Linux and macOS / BSD. Would you merge them if separated out?

By the way, I wonder why you do not use boost for the networking parts?

And the Linux-specific includes would be either moved in a Linux compat directory or be empty dummy files in macOS compat.

I suggest to consider to merge the operating system independent changes 052ad74 and 414de56?

[nmeum]

I suggest to consider to merge the operating system independent changes 052ad74 and 414de56?

Sure, I can cherry-pick those to the master branch directly :)

However, there are some difficulties, e.g. with named and unnamed semaphores, to follow your suggestions.

Could you briefly explain what issues you encountered with the semaphore usage in the UART? The use of semaphores in the UART should adhere to POSIX.1-2008. I would expect it to simply work on any POSIX compliant system.

By the way, I wonder why you do not use boost for the networking parts?

Using the POSIX socket API directly is a bit easier for our simple use cases.

[U2654]

I suggest to consider to merge the operating system independent changes 052ad74 and 414de56?

Sure, I can cherry-pick those to the master branch directly :) 👍

However, there are some difficulties, e.g. with named and unnamed semaphores, to follow your suggestions.

Could you briefly explain what issues you encountered with the semaphore usage in the UART? The use of semaphores in the UART should adhere to POSIX.1-2008. I would expect it to simply work on any POSIX compliant system.

OK. This is indeed a macOS issue, see here.

Nevertheless, named semaphores would not make the code worse.

[U2654]

In summary:

1. f974af5 and 875c2d4 deal the ELF-Section problem and could be resolved. 052ad74 (missing include) and 414de56 (namespace clarification) are platform independent, small changes. b22e3b1 is changed, see comment.

2. e209a09 (byteswap), a5f8117 (x.st_atim -> x.st_atimespec…), 84b3a50 (lseek64), c0ad002 (tun.h), 85a51cf (can.h) can all be resolved if I follow your suggestion with a compat include...

3. b97fa10 (semaphores) is not solvable by a platform specific include. I guess, this could be solved if you would also switch to named semaphores.

4. Finally, 43b8b79 is partly solvable by the include approach (tun.h, udp->dest to uh_dport), but leaves two issues: using ether_header instead of ethhdr and ip instead of iphdr should be acceptable for linux. Thus, one issue would remain, the differences of getting the mac address. I guess that could be solved with define/include by getting the mac using getifaddrs in linux as well: AF_PACKET could be defined as AF_LINK and a define might solve the differences between sockaddr_ll and sockaddr_dl. I have not tried out.

What do you think? Shall I rework this? Or would 3. and 4. be to great changes and not be acceptable for you?

[U2654]

I removed all #ifdef __APPLE__, added a combat directory which contains linux header replacements for building with macOS, decision takes place in the cmakefile. I changed some code which works on both operating systems, see above. Maybe it is worth now considering merging?

[nmeum]

Sorry, I was a bit busy with other things this week.

f974af5 and 875c2d4 deal the ELF-Section problem and could be resolved.

Should be fixed in 8c4132ee01fcca1704ab26fc46048fd90f0ff86d.

052ad74 (missing include) and 414de56 (namespace clarification) are platform independent, small changes.

Cherry-picked these, will be available on GitHub as soon as the repository is mirrored again.

b97fa10 (semaphores) is not solvable by a platform specific include. I guess, this could be solved if you would also switch to named semaphores.

Could you elaborate what exactly the problem with the unnamed POSIX semaphore API is? Do you just get a compilation warning? My understanding is that sem_init and assorted functions are mandatory since POSIX.1-2008 (issue 7) see the change history in sem_init(3) and this bug report. As such, I would expect Mac OS to provide sem_init and assorted functions: What error do you get when compiling this code?

Thus, one issue would remain, the differences of getting the mac address.

Maybe we can just remove the Ethernet peripheral in its entirety. It has some functional issue and the SLIP peripheral basically satisfies the same use case. Will need to discuss this with my colleagues.

What do you think? Shall I rework this? Or would 3. and 4. be to great changes and not be acceptable for you?

Hard to tell, 3. should be fine (see comment above) I think we can also find a solution for 4. but as I laid out in https://github.com/agra-uni-bremen/riscv-vp/pull/8#issuecomment-791305169 Mac OS support is not a priority for us. If the changes are encapsulated in a way that it doesn't really impact existing code much, I wouldn't mind merging this. Unfortunately, that's hard to tell without seeing the final changeset though. It's ultimately up to you.

[U2654]

Well, our comments seem to appear overlapping in time. The final changeset is in e33a7ad.

[nmeum]

Thanks, I can take another look next week. Could you elaborate on the semaphore issue though?

[U2654]

Sorry. Something went wrong. Actually, this should be only the e33a7ad. Everything later should not be here. I don't have a clue how to get them out of here.

[U2654]

I have split all macOS porting things into a macos branch. However, I cannot change this pull request to the macos U2654::macos and I am not sure if some of the verified changes above will vanish if I rebase my master branch to the macos branch. Shall we close this merge request and I do one with the macOS branch?

[U2654]

I close this request. I make another one with the macOS things only.