Edd Barrett <e...@theunixzoo.co.uk> writes: > Hi Greg, > > Thanks for getting back to us. I'm an OpenBSD developer trying to get > the NetBSD driver to work for OpenBSD. > > On Fri, Oct 12, 2018 at 10:02:16PM -0400, Greg Troxel wrote: >> 405: Option not supported by drive > > I've seen this. If I remember correctly (but please double check), I > traced it to setting the drive speed. I think it's harmless. > >> So it seems that moving NetBSD to CD_LBA_FORMAT at best needs more work, >> and that the LBA change should be ifdef'd for OpenBSD for now. > > Great. That's exactly what I've done in my branch. Thanks for trying > this out.
so specifically your ifdef openbsd around this seems ok on netbsd (as built and tested from your branch). >> (If lib/driver/netbsd.c is actually used on OpenBSD, it would be nice to >> have a comment at the front of the file explaining this.) > > The build system will choose the NetBSD backend for OpenBSD, but it > currently doesn't work. ok, but when this is merged a comment is in order, so that people starting to read the code will get that. > If you get time, would you be able to try my branch on NetBSD? The only > outstanding issue for OpenBSD on this branch is cdda-player track > skipping is broken. > > I'd be interested to see if I've introduced regressions. > > https://github.com/vext01/libcdio/tree/openbsd_fixes_to_master I built the branch and am currently using cd-paranoia with it to rip a disc, and I'm getting files with the same hash as before. cdda-player doesn't work, but it doesn't on master either, and it seems roughly the same. It gets errors, lists the tracks, and indicates that it's playing but keyboard input does nothing, and ^C exits. (I understand that I almost certainly have no audio path, but I expect the curses ui to sort of work.) Reading the diff, there are some changes whose comments don't explain the change and why it's necessary (vs sort of restating what I absorb from the diff). Specifically these 8207ace8914c418dea9d626d7839bec0f7202fab 946ebc924ddeb7b26badfdcc7f29f0f5373ff027 a7fc90e1ce7a70a9efde4d1ca4711b20c5b2c8e5 Most of the changes look ok. And then there's the dropping of using sysctl in favor of probing by name - and it's not clear why that should be changed on NetBSD even if the sysctl approach doesn't/can't work on OpenBSD. (It's also not clear why it shouldn't change, but for a big change like that there should be rationale in the commit message.) Some of the changes look unrelated to OpenBSD (unbreak cdda-player?, Ensure that stdin..., missing set of b_cd, add missing -d) Also, I don't get this: ef44a97af6430107f09e27a31576c4adfa36ac09 It does not make sense to me to be compiling the netbsd.c file on operating systems where that isn't the plan, or if it is compiled because ifdefing the whole file is easier than not including it via automake, then a giant ifdef around the file. Hope this helps and if you need me to test more or review code changes please feel free to ask.