Various problems & fixes

[Damian Ivereigh]

Hi guys,

I have been working on the head release and integrating the
back-end tdb stuff such that it talks to our directory service
instead.

While getting it going, I have found the following that you may be
interested in:-

I think a bug in nt_printing.c & srv_spoolss_nt.c - both
files use global_myname rather than local_machine. This means that it
fails if you use netbios aliases (which we do big time) - the wrong
name gets put in various places. I have got away with just doing a
search/replace on these guys and it seems to be fine. However you or
JF may like to check into that. It's easy to reproduce the problem:
just connect to a netbios alias rather than the real name and you'll
see it break when downloading the driver files and even viewing the
properties window.

I have found another problem in the lib/select.c in the function
sys_select(). There seems to be this whole thing around avoiding race
conditions using a pipe. Anyway it doesn't seem to quite work. If for
some reason an extra character appears
in the pipe (that is not represented by the difference between
pipe_read & pipe_written), then the process is sent into an endless
loop (because it never tries to read this character). While this
shouldn't ever happen it does on our machines (about once a day under
heavy load)!

So I put a check in there to force the reading of the character if the
was something waiting to be read. While I was about it I also put a
check in there to make sure that nothing was attempted to be read if
there was no character to be read (which would cause a hang). I am not
so sure about this one. I wonder if a fcntl shouldn't be done on the
read end to set O_NDELAY which would then avoid a hang.

I have enclosed a patch that is my (rather uneducated) fix
(select.c.patch)

Another area which I wouldn't mind you looking at is the all the stuff
around the get_ntforms(), write_ntforms() & delete_a_form() - all in
printing/nt_printing.c. After a form is deleted a "hole" is left in
the tdb file (the index number is also stored). Next time
get_ntforms() is called an uninitialised section of memory is used in
the form list. When this is then written out again (write_ntforms()),
that garbage will be written to the tdb file. This may be enough to
really confuse the clients.

I haven't got a fix for this right now. However I am thinking of
re-doing this anyway, since I would like the stuff written to a
central DB - which may as do the add/update and deletes directly (as
opposed to reading it all in, updating it in memory and then writing
it all out again).

These functions go to great lengths to store the index number in the
record,
however I can't see any good reason why it does this. Does it really
matter
what order the forms are held in the array? Does nt/win2k get upset if
they are ordered differently?

I am wondering if at some point I made the database (i.e. tdb) layer
better extracted so that other db layers could be plugged in more
easily (i.e. replacement of a .c or something like that).

Damian

-- 
Damian Ivereigh
CEPS Team Lead
http://wwwin-print.cisco.com
Desk: +61 2 8446 6344
Mob: +61 418 217 582
-- 
Damian Ivereigh
CEPS Team Lead
http://wwwin-print.cisco.com
Desk: +61 2 8446 6344
Mob: +61 418 217 582
-------------- next part --------------
Index: lib/select.c
===================================================================
RCS file: /cvsroot/samba/source/lib/select.c,v
retrieving revision 1.5
diff -c -r1.5 select.c
*** lib/select.c	2000/11/17 01:05:50	1.5
--- lib/select.c	2001/01/04 04:06:30
***************
*** 72,81 ****
--- 72,96 ----
  
  	if (FD_ISSET(select_pipe[0], fds)) {
  		FD_CLR(select_pipe[0], fds);
+ 		if (pipe_written == pipe_read) {
+ 			/* This should never happen - but its a failsafe
+ 			** to avoid an endless loop if a character is
+ 			** waiting to be read yet the pipe_read/written
+ 			** variables don't show it */
+ 			pipe_written++;
+ 		}
  		ret--;
  		if (ret == 0) {
  			ret = -1;
  			errno = EINTR;
+ 		}
+ 	} else {
+ 		if (pipe_written != pipe_read) {
+ 			/* This should also never happen - but its a failsafe
+ 			** to avoid an endless wait if there is no character
+ 			** waiting to be read yet the pipe_read/written
+ 			** variables says there is */
+ 			pipe_read=pipe_written;
  		}
  	}