[paranoia] CD Paranoia for MacOS X

[Scott Wood]

On Dec 8, 2003, at 12:39 PM, Peter Jones wrote:
> Hey, some comments.
>
> 1) Is it really worth doing ENDIAN_H in configure.in instead of just
> "#ifdef __APPLE__"'ing it in the two places it gets used?

I believe machine/endian.h is a general BSDism, and not specific
to Apple.

>  Same question
> applies for GETOPT_H -- it's used at 1 place in actual C code.  Would
> "#ifndef __APPLE__" around that include not be just as reasonable?

getopt.h and getopt_long() are GNU extensions (albeit ones which have
been copied by some non-GNU implementations, including the latest 
release
of OS X).  It's not an Apple-specific workaround.

>  It
> would certianly look more like the normal way people do that in C code.
> Maybe I just hate autoconf though :)

I don't particularly care for it myself, though my distaste is more for
its implementation (that, and users of it who check for everything under
the sun regardless of whether they can handle failure (are things like
strcpy() really going to be missing?)) than what it's trying to 
accomplish
(it's slow, and  debugging an auto-generated multi-thousand line shell
script sucks).

> +ifeq (@host_os@,linux)
> +SHLIB := so.0.$(VERSION)
> +COMPATLIB := so.0
> +LINKLIB := so
> +else
> +ifeq (@host_os@,darwin)
> +SHLIB := 0.$(VERSION).dylib
> +COMPATLIB := 0.dylib
> +LINKLIB := dylib
> +else
> +How do I make shared libraries?
> +endif
> +endif
>
> This looks like it'd be safe to default to the Linux way; most UNIXes
> follow a similar convention, don't they?

Probably.

> Along the same lines, you seem to _add_ a lot of "it's either MacOS or
> Linux" assumptions.

Most of that is based on it appearing to only run on Linux at present
(I hadn't heard of the FreeBSD port), and I didn't know of anything
better to #ifdef on.  As the README says, it's a quick hack. :-)

>   AFAIK (and I could be wrong) current cdparanoia also
> works on e.g. FreeBSD.  I don't know how much cdparanoia in ports is
> patched, but I find it likely that, for example, FixupTOC here:
>
> +#ifdef __linux__
>    if (d->ioctl_fd != -1) {
> +    struct cdrom_multisession ms_str;
>      int result;
>
>      ms_str.addr_format = CDROM_LBA;
>      result = ioctl(d->ioctl_fd, CDROMMULTISESSION, &ms_str);
>      if (result == -1) return -1;
>
>      if (ms_str.addr.lba > 100) {
>
> works without your modification on a FreeBSD box, but doesn't work
> afterwards.  Do we know which platforms a change like that is 
> affecting?
> If we don't, "#ifndef __APPLE__" seems far more appropriate than 
> "#ifdef
> __linux__".

Agreed.  Ideally, stuff like this would be hidden in the low-level
interface code, but I guess that'll have to wait for cdparanoia IV...

> All that being said, most of the changes look pretty reasonable.  One
> relatively serious issue I have is that you've changed 
> cdda_interface.h's
> struct cdrom_drive.
>
> Obviously, you've almost certainly got to do this, but the downside is
> that it changes the ABI, and so getting a new version into linux 
> distros
> means rebuilding ripping software such as grip which links against
> libcdda_interface, and means that the changes can't go into any 
> commercial
> distros that try to guarantee ABI compatibility where possible across
> minor releases.

The only change that isn't __APPLE__-specific is the close method, which
could be moved to the end of the struct.  That way, compatibility is 
only
affected if apps need to know the size of the struct.  Is that currently
the case (if so, it could be #ifdeffed as well)?

> One last note.  You appear to have taken a really, really old snapshot.

I downloaded the latest stable release.  I guess I took the website too
literally when it said that CVS "Currently matches 9.8 release". :-P

-Scott

--- >8 ----
List archives:  http://www.xiph.org/archives/
Paranoia homepage: http://www.xiph.org/paranoia/
To unsubscribe from this list, send a message to 'paranoia-request at xiph.org'
containing only the word 'unsubscribe' in the body.  No subject is needed.
Unsubscribe messages sent to the list will be ignored/filtered.