[sane-devel] Patches for SANE 1.0.9 be/fe compiled on IRIX
Peter Fales
peter@fales-lorenz.net
Mon, 6 Jan 2003 19:23:08 -0600
The patches to dc210.c, dc240.c, dc210.conf, dc240.conf, dc25.conf, and
gphoto2.conf look reasonable so I've pulled them into CVS.
The changes to dc240.h aren't needed because I don't depend on packed
directory structures any more.
The patches to backend/Makefile.in and acinclude.m4 seem to work OK, but
I'll let Henning decide whether to accept those.
--
Peter Fales
On Tue, Jan 07, 2003 at 12:46:32AM +0100, Andrea Suatoni wrote:
> 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
>
> _______________________________________________
> Sane-devel mailing list
> Sane-devel@www.mostang.com
> http://www.mostang.com/mailman/listinfo/sane-devel