[sane-devel] Patches for SANE 1.0.9 be/fe compiled on IRIX

[Andrea Suatoni]

Henning Meier-Geinitz wrote:

> > In order to compile properly the two packages,
> 
> Was there anything that didn't compile at all? I don't ahve access to
> Irix any longer, so the last version tested was 1.0.8.

Nothing specific, just that the SGI freeware is built with the SGI MIPSpro
C/C++ compilers (not with GCC) and installed in a specific non standard place
(/usr/freeware and below). There are many warnings during compilation (SGI
compilers can be very picky) that I shut down in my packaging Makefile (not
sent to this list, because it not interesting for you).

I'm currently away from my SGI boxes 'till the end of this week, so I have no
access to the source code right now. However, I'll try to answer to your
questions based on what I remember.

> > +++ sane-backends-1.0.9-patched/backend/Makefile.in   Thu Dec 12 21:55:32 2002
> > @@ -46,6 +46,7 @@
> >  CFLAGS = @CFLAGS@
> >  LDFLAGS = @LDFLAGS@
> >  BACKENDLIBS = @LIBS@ @DL_LIB@
> > +GPHOTO2_LIBS = @GPHOTO2_LIBS@
> >  DEFS = @DEFS@
> >
> >  LIBTOOL = ../libtool
> > @@ -135,6 +136,11 @@
> >
> >
> >  .PHONY: all clean depend dist distclean install uninstall
> > +
> > +libsane-gphoto2.la: gphoto2.lo gphoto2-s.lo $(EXTRA) $(LIBOBJS)
> > +     @$(LIBTOOL) $(MLINK) $(CC) -export-dynamic -o $@ $($*_LIBS) \
> > +     $(LDFLAGS) $(GPHOTO2_LIBS) $(BACKENDLIBS) $^ -rpath $(libsanedir) \
> > +     -version-info $(V_MAJOR):$(V_REV):$(V_MINOR)
> >
> >  libsane-%.la: %.lo %-s.lo $(EXTRA) $(LIBOBJS)
> >       @$(LIBTOOL) $(MLINK) $(CC) -export-dynamic -o $@ $($*_LIBS) \
> 
> What's the reason for the gphoto2 changes and the separate rule for
> building the library?

There is a specific reason, in that I didn't want to make the whole
sane-backends package dependent from gPhoto2. The IRIX packages are built in
subsystems, and each subsystem have its own set of package dependencies
(called prerequisites in SGI terminology). A given subsystem can be installed
only if all its prerequisistes are satisfied. Differently from RPM (or a
similar package concept), an IRIX package is normally one distribution file (a
.tardist), internally subdivided in subsystems.
In other words, while with RPM you have package.rpm, package-devel.rpm, etc,
with IRIX inst images you have package.tardist, containing all the subsystems
at once (bin, lib, relnotes, man, etc).

In the case of gPhoto2, without the above modification I would be forced to
make the whole sane-backends package dependent from the gPhoto2 libraries.
That is, the user would be forced to install also the libraries subsystem from
the gPhoto2 packages, even if he/she has no digital camera. Since the only
SANE backend that needs the gPhoto2 libraries is the gPhoto2 backend, I
modified the config rules so that libgphoto2 is only linked with this specific
backend, and not with all the other SANE backends. In this way, I can move the
SANE gPhoto2 backend to a separate subsystem, which depends from the gPhoto2
libraries. If the user doesn't need the gPhoto2 features, he/she will be
nevertheless able to install the remaining SANE backends without be forced to
install also the gPhoto2 libraries.

Additionally, depending from the way the runtime linker loader is implemented
in the various OS, forcing the gPhoto2 libraries to be linked with all the
SANE backends could require the load and symbols resolution of that library at
run time (that is, except in the case of the SANE gPhoto2 backend, all the
backends potentially consume more memory than necessary).

> > diff -ruN sane-backends-1.0.9/backend/as6e.c sane-backends-1.0.9-patched/backend/as6e.c
> > --- sane-backends-1.0.9/backend/as6e.c        Tue Dec  5 20:10:20 2000
> > +++ sane-backends-1.0.9-patched/backend/as6e.c        Thu Dec 12 21:56:54 2002
> > @@ -604,8 +604,7 @@
> >         return (SANE_STATUS_GOOD);
> >       }                       /*else */
> >      }
> > -  else
> > -    return (SANE_STATUS_IO_ERROR);
> > +  return (SANE_STATUS_IO_ERROR);
> >  }
> 
> Is this to fix a warning or is this a real change in code?

It's just that in the original form the SGI compiler will issue a warning
because it sees the function not returning a value. As I've said, the compiler
can be very picky. You can omit this change if you want, even if avoiding the
unnecessary else makes the code more readable (IMHO; of course ;)

> > +++ sane-backends-1.0.9-patched/backend/canon-sane.c  Thu Dec 12 21:58:46 2002
> > @@ -1798,7 +1798,7 @@
> >    SANE_Status status;
> >    SANE_Byte *out, *red, *green, *blue;
> >    SANE_Int ncopy;
> > -  size_t nread, i, pixel_per_line;
> > +  size_t nread = 0, i, pixel_per_line;
> >
> >    DBG (21, ">> read_fb620\n");
> >
> 
> That's because of the DBG that prints nread before initializing it?
> The correct fix is to move the DBG after the setting nread, I guess.

The SGI compiler warned me that the variable nread was used before it was
initialized, hence I set it to 0. You are probably right in reordering the
sequence of statements, but not always I have the time to go deep in the code.
This was one of those cases.

> > +++ sane-backends-1.0.9-patched/include/sane/config.h.in      Thu Dec 12 23:36:53 2002
> > @@ -393,3 +393,13 @@
> >
> >  /* Define to `unsigned long' if <sys/types.h> does not define. */
> >  #undef u_long
> > +
> > +/* Define a the function prototypes if these function are missing. */
> > +#ifndef HAVE_STRSEP
> > +char *strsep(char **stringp, const char *delim);
> > +#endif
> > +
> > +#ifndef HAVE_STRNDUP
> > +#include <sys/types.h>
> > +char *strndup(const char * s, size_t n);
> > +#endif
> 
> This will be removed automatically with the next run of autoheader. If
> it makes sense to add these declarations, I can put the code for
> generation in configure.in.

AFAIR, it was a quick hack to give the compiler the required prototypes for
one or two C sources. If possible, I try to not recreate the GNU auto stuff
unless strictly required. I seem to recall that the reason for the above was
that these functions had no corresponding prototypes in any of the distributed
header files.

> > diff -ruN sane-frontends-1.0.9/src/xcam.c sane-frontends-1.0.9-patched/src/xcam.c
> > --- sane-frontends-1.0.9/src/xcam.c   Sat Jun  9 14:52:05 2001
> > +++ sane-frontends-1.0.9-patched/src/xcam.c   Fri Dec 13 00:41:42 2002

> That one doesn't work on Linux/X11/i386. The colors are wrong. The old
> code was correct (at least for 24 bit, 4 bytes per pixel, little
> endian). Well, at least it worked here :-)

Using the SGI X server in 24 bit (SGI boxes are big endian), the red and blue
channels are swapped without the change. You may enclose the changes I've
applied using #ifdef __sgi (this will be valid only on SGI machines, using
either GCC or the SGI compiler).

> I'm fascinated that someone actually cares about xcam :-)

Well, once you have built it, you have to test it before passing the package
to the SGI folks. Incidentally, my Kodak DC240 digital camera displayed all
its pictures with the colors channels swapped, so I applied that quick hack.
Having that camera, I've also included a set of patches for the SANE DC240
backend, which was not inited properly on IRIX (there are comments explaining
that the serial port initialization code is inspired by the corresponding
driver in gPhoto2 libraries). I've applied the same changes to the SANE DC210
and DC25 backends, because the code was similar, but since I have no such
cameras, I cannot test them.

Andrea