Re: [Libcdio-devel] problems with libcdio on openbsd (and probably netbsd too)

[Rocky Bernstein]

Thanks for your interest in libcdio and your comments. It looks like yes
the error messages should be turned in to warnings.

Would you work up a patch incorporating Thomas' (and others ideas if they
should subsequently appear) and send it?

As for how to send it, I am not sure that savannah.gnu.org something as
nice as  a git "pull/merge" request. But if you did this in git you can use
"git format-patch" or "git diff" . And of course there is also "diff -u" if
was not done in git. But in "diff" please list the old version first.

Then add the patch to
https://savannah.gnu.org/patch/?func=additem&group=libcdio  I'll put it in
a branch which will allow others to more easily review and then from there,
we'll merge it into the main branch.

Thanks

On Sat, May 26, 2018 at 2:44 PM, Edd Barrett <address@hidden> wrote:

> Hi libcdio developers!
>
> I'm an OpenBSD developer trying to get our ancient libcdio package up to
> date. Having spent some time debugging, I've found two problems.
>
> 1) The device detection logic is broken.
>
> On openbsd, the netbsd driver is used. To find available drives, libcdio
> parses the output of a sysctl to get the available CD drives, but the
> netbsd format of this string differs from openbsd. I've patched it
> locally with code which simply finds devices by opening /dev/rcd[0-9]c.
> That would probably work on netbsd too.
>
> 2) Error and warning handling in netbsd.c seems wrong.
>
> If you have two CD drives, the first is empty, the second contains the
> disk you want to access, cdio_get_devices_with_cap() will simply exit().
>
> cdio_get_devices_with_cap() is a loop which queries devices in order. It
> calls cdio_get_first_track_num(), then get_first_track_num_netbsd() and
> finally _cdio_read_toc(), where you have:
>
> ---8<---
>           res = ioctl(_obj->gen.fd, CDIOREADTOCHEADER, &_obj->tochdr);
>
>           if (res < 0) {
>
>                   cdio_error("error in ioctl(CDIOREADTOCHEADER): %s\n",
>
>                              strerror(errno));
>
>                   return false;
>
>           }
> --->8---
>
> When the empty /dev/rcd0c is queried, the ioctl returns an error, and
> cdio_error() exits. I think this is wrong. I'm testing using audacious,
> and it just disappears if a drive is empty. Audacious never had a chance
> to show the user an error message! I'd expect libraries to never exit()
> unless it really can't continue.
>
> If I comment the call to cdio_error(), I can play a CD from the second
> drive using audacious. The `return false` even suggests that the author
> of this code was expecting cdio_error() to return.
>
> I suspect it's broken for NetBSD too (but I have not tried). They have a
> patch in their tree which stops cdio_error() from exiting!
> http://cvsweb.netbsd.org/bsdweb.cgi/pkgsrc/misc/
> libcdio/patches/patch-aa?rev=1.5&content-type=text/x-cvsweb-markup
>
> There are a ton more calls to cdio_error() in netbsd.c, many of them
> look like they too should not stop the program, but maybe be downgraded
> to cdio_warn(), or even be removed(?). By comparison, there isn't a
> single call to cdio_error() in gnu_linux.c.
>
> I wonder if one of the core libcdio developers could comment? Providing
> that an appropriate error is returned, should all of the cdio_error()s
> be at least cdio_warn()s?
>
> Thanks!
>
> --
> Best Regards
> Edd Barrett
>
> http://www.theunixzoo.co.uk
>
>