Re: [Libcdio-devel] CD_MSF_FORMAT vs LBA on NetBSD

[Thomas Schmitt]

Hi,

the commit history in libcdio/commits/openbsd_fixes_to_master_for_merge
looks good to me, as far as i can judge without testing on OpenBSD and
NetBSD.

A last bug fix about CDIOREADMSADDR in get_last_session_netbsd() seems
still necessary (i proposed to initialize "int addr" to 0).

I will review libcdio-paranoia/commits/start-track-num-not-one
later.

---------------------------------------------------------------------------
In detail:

I begin my curious inspection at
  
https://github.com/vext01/libcdio/commit/8207ace8914c418dea9d626d7839bec0f7202fab
of june 11 2018.

"OK"   means that i am fully convinced.
"(OK)" means that it looks good to me but i am not qualified to judge all
       aspects.

- 8207ace8914c418dea9d626d7839bec0f7202fab
  "Make autogen.sh work on OpenBSD."
  (OK)
  On how many operating systems was this this tested ?
  I ran it on Debian "oldstable" without noticing problems.

- 946ebc924ddeb7b26badfdcc7f29f0f5373ff027
  "OpenBSD/NetBSD: libcdio_error -> libcdio_warn."
  OK
  The new error reporting matches the one in gnu_linux.c.

- 0897e98ba338fa1223a4b7ff786bf517362ff34b
  "Improve NetBSD device enumeration and make it work on OpenBSD."
  (OK)
  The new drive enumeration by /dev/rcd%d%c matches in principle the one
  in libburn/sg-netbsd.c. libburn uses {'d', 'c'} for "%c" instead of
  'a'+RAW_PART..
  (Advise is welcome about how bad this hardcoding in libburn might be.)

- 3826afab31d4d418be3f0fee1132fa61077000f5
  "Transplant missing functions from OpenBSD ports into the NetBSD driver."
  (Not much opinion)
  It looks partly outdated (get_track_lba_netbsd() exists) and it introduces
  the suspected reason for the error message about CDIOREADMSADDR.

- a7fc90e1ce7a70a9efde4d1ca4711b20c5b2c8e5
  "Unbreak cdda-player -s."
  (No opinion)

- 9c160059634ec85c2880ab1bd6cf00012ac02a21
  "Ensure that stdin is in the fd set for select_wait()."
  (No opinion)

- a4d9265e1fe1aaec9cce28a154ef0b0a6cec03da
  "Missing set of b_cd for stopping the CD non-interactive."
  (No opinion)

- 11a35398f08e83377ce26b30c348b83821dfeac8
  "Add missing `-d` to cdda-player help message, and fix an indent."
  OK

- e2f3919357b76d9fadb3d57f20d2b1e8198f32d1
  "The TOC needs to be read in LBA format on OpenBSD."
  (Not in effect any more)
  This was a wrong move.

- ef44a97af6430107f09e27a31576c4adfa36ac09
  "Guard some functions that should only be built on Net/OpenBSD. "
  (No opinion)
  I am surprised that netbsd.o is built on Linux at all. I would be
  even more surprised if it was linked into libcdio on Linux.

- 4dab9d84e305771cebe202810e484a239fed4963
  "Use MSF throughout the NetBSD driver."
  (OK)
  This was the right move instead of e2f3919.

- d04c48e36e7bb810dede415950a8cdf69e625e0a
  "NetBSD/OpenBSD: Do not assume the first track number is 1."
  (Partly OK)
  This started the hunt for bugs with track number > 1.
  A small bug in this change was addressed in the next commit.

- e037be76aef45d7a6181b4eee731384b372b0575
  "Revert "cd-info: don't assume that the first track number is 1.""
  (Partly OK)
  The commit title is wrong. It does not revert e037be7 but rather
  fixes the mentioned small bug in that commit.
  But it is buggy by old the condition "if (i == i_tracks)", which gets
  fixed by merge 4ddd64f504a54a8b6d079f5fb17f82054f05b452.

- 539c52488506c5bacdfebbcafc5406034a0738fd
  "Fix leadout computation."
  OK

- 561d7a8ed195e29a16cf87236c568531c1871031
  "cd-info: fix track list header. "
  OK

- cc9f216badadf097f5c33976fb082808a9ea01a4
  "netbsd.c: Fix invalid error return in get_track_msf_netbsd()."
  OK

---------------------------------------------------------------------------

Have a nice day :)

Thomas