Bugs & Fixes: FreeBSD, Snapscan, Sane 1.01-4

=?ISO-8859-1?Q?Mikko_Ty=F6l=E4j=E4rvi?= (mikko@securitydynamics.com)
Tue, 6 Apr 1999 08:03:27 +0200 (MET DST)

Hi!

I finally got around to changing SCSI-cards and testing if my scanner
still worked after moving to FreeBSD 3.1. Of course it didn't...
But after a few sessions with truss and gdb, I can now scan again.

For the records:

FreeBSD 3.1, Adaptec 1542B, AGFA Snapscan 310, sane-pre1.01-4

Regards,
/Mikko

Problem 1:

The snapscan backend does not work with FreeBSD 3.1 (the CAM SCSI
stuff). The reason is that CAM uses a faked fd that has nothing in
common with a unix file descriptor. Consequently doing a dup2() on
it will not work too well.

As far as I can see there is no reason to move the scanner to stdin in
the child process, and just removing the operation indeed made
scanning work.

NOTE: It was possible to scan with "scanimage", but that is just
coincidence (the CAM device happens to be at index 0 == STDIN_FILENO)

A quick grep seems to indicate that no other backend suffers
from this quirk.

--- backend/snapscan.c.org Mon Apr 5 14:26:47 1999
+++ backend/snapscan.c Mon Apr 5 14:35:11 1999
@@ -2843,10 +2843,8 @@
and stdout the pipe */
signal (SIGTERM, handler);
reader_pss = pss;
- dup2 (pss->fd, STDIN_FILENO);
dup2 (pss->rpipe[1], STDOUT_FILENO);
close (pss->rpipe[0]);
- pss->fd = STDIN_FILENO;
reader (pss);
/* regular exit will cause a SIGPIPE */
DBG (DL_MINOR_INFO, "Reader process terminating.\n");

Problem 2:

The CAM code will overflow when using "xscanimage". Every time a
device is opened, a new slot in a bounded array is allocated. It is
never re-used, even when the device is closed. A few previews and
scans in the same "xscanimage" process is all it takes to use up all
the slots, and start writing outside allocated memory.

There is no need to copy the data from cam_open_device(), it is
already allocated, and will be freed by cam_close_device(). In fact,
copying the data will only result in a memory leak.

Minor problem 2.1:

The cam_open_device() function requires access to /dev/xpt0, rather
than (for example) /dev/pass4, where the scanner is, as well as the
device being called "pass<N>".

Using cam_open_pass() instead goes directly for the pass device,
saving users the headache of figuring out why it will only work when
running as "root", even though anyone can read & write /dev/pass4, or
why it will not work with a symlink.

--- sanei/sanei_scsi.c.org Mon Apr 5 14:26:35 1999
+++ sanei/sanei_scsi.c Mon Apr 5 14:58:17 1999
@@ -197,7 +197,7 @@

#if USE == FREEBSD_CAM_INTERFACE
# define CAM_MAXDEVS 128
-struct cam_device **cam_devices = NULL;
+struct cam_device *cam_devices[CAM_MAXDEVS] = { NULL };
#endif

static struct
@@ -817,23 +817,22 @@
just '{' unfortunately, this only works if all of
the '{' are that way. */

- static int unit = -1;
- struct cam_device* curdev = NULL;
+ struct cam_device *curdev;

- fake_fd = -1;
+ fake_fd = 1;
fd = -1;

- if(cam_devices == NULL) {
- size_t ary_sz = sizeof(struct cam_device*) * (CAM_MAXDEVS + 1);
- cam_devices = malloc(ary_sz);
- bzero(cam_devices, ary_sz);
- unit = -1; /* redundant, I know. */
- }
+ if((curdev = cam_open_pass(dev, O_RDWR, NULL)) != NULL) {
+ for (fd = 0; fd < CAM_MAXDEVS && cam_devices[fd] != NULL; fd++)
+ ;

- if((curdev = cam_open_device(dev, O_RDWR)) != NULL) {
- cam_devices[++unit] = malloc(sizeof(struct cam_device));
- bcopy(curdev, cam_devices[unit], sizeof(struct cam_device));
- fd = unit;
+ if (fd == CAM_MAXDEVS)
+ {
+ DBG(1, "sanei_scsi_open: too many CAM devices (%d)\n", fd);
+ cam_close_device(curdev);
+ return SANE_STATUS_INVAL;
+ }
+ cam_devices[fd] = curdev;
}
else {
DBG(1, "sanei_scsi_open: device %s not found\n",
@@ -969,7 +968,8 @@
close (fd);

#if USE == FREEBSD_CAM_INTERFACE
- cam_close_device(cam_devices[fd]);
+ cam_close_device(cam_devices[fd]);
+ cam_devices[fd] = NULL;
#elif USE == DOMAINOS_INTERFACE
{
static int index;
@@ -1689,7 +1689,7 @@
char* data_buf;
size_t data_len;

- if (fd < 0) {
+ if (fd < 0 || fd > CAM_MAXDEVS || cam_devices[fd] != NULL) {
fprintf(stderr, "attempt to reference invalid unit %d\n", fd);
return SANE_STATUS_INVAL;
}

Mikko Tyo"la"ja"rvi________________________________mikko@securitydynamics.com
SecurityDynamics

--
Source code, list archive, and docs: http://www.mostang.com/sane/
To unsubscribe: echo unsubscribe sane-devel | mail majordomo@mostang.com