[Desktop_printing] CUPS code quality ...

[Norm Jacobs]

It also sounds like asynchronous interfaces for print queue enumeration,
status query, and job submission are likely to be of interest in a future
revision.

       -Norm

McDonald, Ira wrote:

>Hi,
>
>[for context] PAPI is the FSG Open Printing standard PAPI/1.0 (July
>2005) at:
>
>ftp://ftp.pwg.org/pub/pwg/fsg/spool/papi-v1.0-2005-07-15.pdf
>
>There have been complete open source reference implementations for
>several years.  Norm Jacobs (Sun, chair of FSG Open Printing) has
>PAPI supporting IPP and LPR on Solaris and is the principal current
>  
>
FWIW, It also builds and runs on Linux.  I suspect that the same
could be said for several platforms with POSIX interfaces.

>maintainer of the PAPI specification and source.
>
>Input for extensions to PAPI/1.0 is welcome (we're looking at
>Capabilities now, which means PPD parsing, etc.).
>
>
>Cheers,
>- Ira (for FSG/OP International Steering Committee)
>
>
>Ira McDonald (Musician / Software Architect)
>Blue Roof Music / High North Inc
>PO Box 221  Grand Marais, MI  49839
>phone: +1-906-494-2434
>email: imcdonald at sharplabs.com
>
>  
>
>>-----Original Message-----
>>From: desktop_printing-bounces at lists.osdl.org
>>[mailto:desktop_printing-bounces at lists.osdl.org]On Behalf Of michael
>>meeks
>>Sent: Thursday, February 02, 2006 12:24 PM
>>To: Michael Sweet
>>Cc: desktop_printing at lists.osdl.org
>>Subject: Re: [Desktop_printing] CUPS code quality ...
>>
>>
>>Hi Michael,
>>
>>	Good name incidentally :-)
>>
>>On Thu, 2006-02-02 at 08:35 -0500, Michael Sweet wrote:
>>    
>>
>>>I'll address these points in reverse:
>>>      
>>>
>>	Well, it seems you're at beginning to consider some of 
>>these issues in
>>1.2 that's great.
>>
>>    
>>
>>>     1. Thread safety; CUPS 1.2 is thread-safe, and 
>>>      
>>>
>>threading support
>>    
>>
>>>        is enabled by default.  Previous releases were 
>>>      
>>>
>>thread safe if
>>    
>>
>>>        you stayed away from the convenience functions that 
>>>      
>>>
>>used shared
>>    
>>
>>>        global data.
>>>      
>>>
>>	Ok - well, I guess thread local storage is one way 
>>around returning
>>const static internal buffers - although the gotcha for the 
>>unwary still
>>remains. Clearly having a unified, standard calling convention for
>>methods helps substantially wrt. learning the API, not 
>>leaking/crashing
>>and auto-generated scripting bindings to name a few.
>>
>>    
>>
>>>     2. Synchronous; only if you use the convenience APIs - 
>>>      
>>>
>>we use the
>>    
>>
>>>        CUPS API as the basis for cupsd, and all of the HTTP and IPP
>>>        APIs operate as FSMs so that we can handle multiple 
>>>      
>>>
>>connections
>>    
>>
>>>        asynchronously.  Other apps can do this or use 
>>>      
>>>
>>threads as they
>>    
>>
>>>        like.
>>>      
>>>
>>	So - can you write me a short code-fragment, with error 
>>checking,
>>partial write support etc. that would asynchronously fetch a PPD ? (no
>>commenting necessary ;-) [ also some idea of how to integrate 
>>that into
>>a custom mainloop - just need a socket to poll on etc. ] [ 
>>I'm assuming
>>here there is an API function to handle a partial read in 
>>response to a
>>POLLIN|POLLPRI input, and O_NONBLOCK socket handling ? ]
>>
>>    
>>
>>>     2a. Variable time; them's the breaks, some people have fast
>>>         systems, some slow, some people use local servers, some
>>>         remote, some people have 1 printer, some have 10000.
>>>      
>>>
>>	Sure - so clearly synchronous APIs are a disaster 
>>waiting to happen -
>>particularly without threading support; but of course this is 
>>now fixed
>>in 1.2 :-) [ which I don't seem to have on my bleeding-edge 
>>system: odd,
>>so if you don't mind I'll refer below to what I see in 1.1 ].
>>
>>    
>>
>>>     4. Badly designed; also a matter of opinion, although 
>>>      
>>>
>>I heartily
>>    
>>
>>>        agree that certain parts of the API are showing 
>>>      
>>>
>>their age and
>>    
>>
>>>        could use a redesign.  This, unfortunately, is a challenge
>>>        with the parallel goal of maintaining backwards 
>>>      
>>>
>>compatibility
>>    
>>
>>>        in the 1.x series, but we welcome feedback in improving the
>>>        existing APIs and adding new ones that match 
>>>      
>>>
>>developer usage.
>>
>>	Of course back-compat (particularly with this legacy) 
>>is a pain; but -
>>then the problems appear to be legion - an entirely new API (perhaps
>>built on top of CUPS initially) would not have this problem of course.
>>
>>    
>>
>>>You *do* know that you can do a CUPS_GET_PRINTERS request with a
>>>non-blocking HTTP connection to collect the data as you go along,
>>>right?  And use requested-attributes to reduce the amount of data
>>>that is processed?
>>>      
>>>
>>	No - I have no idea; it's not clear from the headers, 
>>it's not clear to
>>me how to do it. When I read the code in days of yore & the 
>>IPP spec. it
>>was not clear to me how to construct arbitrary
>>strings/URIs/etc./whatever & use the raw CUPS http code to achieve
>>anything.
>>
>>    
>>
>>>Comments like this are not useful.  Simple documentation = easier to
>>>follow code (and clearly you are able to follow the CUPS 
>>>      
>>>
>>code, even if
>>    
>>
>>>you don't like the design...)
>>>      
>>>
>>	I spend my life reading code of often very mixed 
>>quality - some of the
>>worst is unfortunately my own :-) perhaps you'd accept a patch to CUPS
>>from me ? ;-)
>>
>>    
>>
>>>We use static and dynamic buffers when they make sense.
>>>Sockets are a requirement for using IPP over HTTP.  
>>>      
>>>
>>	When does a static buffer make sense ? seriously; this 
>>is my point -
>>network traffic is slow, IPC is slow - strdup() is 
>>blindingly, amazingly
>>fast. My point is - why use static buffers all over the place 
>>(which may
>>give some marginal speed win) when they also tend to have well known
>>security issues and of course introduce arbitrary fixed 
>>limits all over
>>the show: thus either wasting memory unnecessarily or limiting
>>longer-string corner cases.
>>
>>    
>>
>>>It is synchronous by design, but nothing prevents you from using the
>>>HTTP and IPP APIs to do this asynchronously - you'll just need to
>>>do the IPP queries and then the HTTP GET yourself.
>>>      
>>>
>>	So again; when I tried to do this some year ago - it 
>>was extremely
>>unclear how to do that; but perhaps I was just missing 
>>something simple.
>>
>>    
>>
>>>>	Of course, what the contract is wrt. this file - 
>>>>        
>>>>
>>whether you get to
>>    
>>
>>>>unlink it later at your leisure, or what is being really 
>>>>        
>>>>
>>passed to you
>>    
>>
>>>>is (AFAICS) not explicit in the commenting. An opaque handle with
>>>>clearly defined & consistent lifecycle semantics would clearly be
>>>>better.
>>>>        
>>>>
>>>We're not trying to define a whole new object/abstraction interface
>>>here (i.e. no glib or similar frameworks), just trying to get the
>>>job done.
>>>      
>>>
>>	Well; I'm not asking for rocket science here - just 
>>expecting some of
>>the hard-learned lessons of maintaining stable ABIs over long 
>>periods to
>>hit home.
>>
>>	So - lets take another example:
>>
>>typedef struct
>>{
>>  int			fd;		/* File descriptor for 
>>this socket */
>>  ...
>>  char			fields[HTTP_FIELD_MAX][HTTP_MAX_VALUE];
>>  ...
>>  /**** New in CUPS 1.1.19 ****/
>>  fd_set		*input_set;	/* select() set for 
>>httpWait() */
>>  http_status_t		expect;		/* Expect: header */
>>  char			*cookie;	/* Cookie value(s) */
>>  /**** New in CUPS 1.1.20 ****/
>>  char			authstring[HTTP_MAX_VALUE],
>>					/* Current 
>>Authentication value */
>>			userpass[HTTP_MAX_VALUE];
>>					/* Username:password string */
>>  int			digest_tries;	/* Number of tries for 
>>digest auth */
>>} http_t;
>>
>>	So several things to say here: exposing all this lot:
>>
>>	*  gives you no guarantees of encapsulation, anything can 
>>	   screw with any of the fields at any time in any thread,
>>	   in uncontrolled ways; resulting in inconsistent state.
>>
>>	*  gives you expansion problems: yes, I'm sure you're not
>>	   expecting people to stack/heap allocate this structure 
>>	   themselves (when compiling with some old headers) - but
>>	   someone, somewhere is going to do this if they can.
>>
>>	*  gives you hard-to-detect versioning problems; if I
>>	   compile vs. 1.1.20 and run with 1.1.19 - and try to
>>	   access 'authstring' which was there in the header when
>>	   I compiled: what happens ?
>>		* the alternative being a linker error when an 
>>		  non-available accessor is called; or a named / 
>>		  enumerated property is not available.
>>
>>	A far better way is to have an opaque structure, and either
>>named/enumerated properties - or individual accessors. If you're
>>particularly 31337 you can not dup strings and return 'const char*'
>>pointers to the internal representation, thus saving a 
>>handful of cycles
>>and exposing yourself to a different set of problems later :-)
>>
>>	Things like:
>>
>>  char			fields[HTTP_FIELD_MAX][HTTP_MAX_VALUE];
>>
>>	are particularly odd; HTTP_FIELD_MAX is 28 for me ( the 
>>end of an
>>enumeration ) of course - it's not really possible to extent that
>>enumeration without breaking the entire http_t layout ;-) but 
>>that aside
>>we have:
>>
>>#  define HTTP_MAX_URI		1024	/* Max length 
>>of URI string */
>>#  define HTTP_MAX_HOST		256	/* Max length 
>>of hostname string */
>>#  define HTTP_MAX_BUFFER	2048	/* Max length of data buffer */
>>#  define HTTP_MAX_VALUE	256	/* Max header field 
>>value length */
>>
>>	So fields consumes 7kb, and other bits perhaps another 
>>2 1/2k or so,
>>size-wise not important, but most of that will be wasted most of the
>>time.
>>
>>    
>>
>>>>	Is this symptomatic of a wider problem ? no idea. All I 
>>>>        
>>>>
>>know is my
>>    
>>
>>>>experience of using the client code & trying to make it do what is
>>>>necessary has been poor in the extreme: of course, it's entirely
>>>>possible that is just down to my ignorance.
>>>>        
>>>>
>>>In my experience, too many developers jump to conclusions about
>>>software before asking the people who wrote the software...
>>>      
>>>
>>	Well - fine; what is your ABI compatibility strategy 
>>here ? why is
>>there no encapsulation/data-hiding ? why are there all these arbitrary
>>limits around the place ? can a FIELD really not contain a 
>>URI ? ( field
>>max 256chars, URI max 1024 chars ).
>>
>>	Of course perhaps my critique is unfair - I've only had 
>>to use the API
>>in quite a minor way; but the above are my questions from reading the
>>http.h header; from a quick scan similar problems are elsewhere too.
>>
>>    
>>
>>>There is already an API like this: PAPI.  However, you may be
>>>disappointed to discover that PAPI's APIs are all 
>>>      
>>>
>>synchronous, with no
>>    
>>
>>>possibility of asynchronous operation.  PAPI also suffers from the
>>>"least common denominator" syndrome, but that will likely 
>>>      
>>>
>>improve over
>>    
>>
>>>time...
>>>      
>>>
>>	Interesting; of course - as you see I'm almost totally 
>>ignorant of
>>printing as a domain :-) perhaps PAPI is what I'm looking 
>>for. ie. if I
>>was going to bless an API/ABI to have to support for the next 
>>20 years &
>>remain compatible with - I would not be picking CUPS in it's current
>>state. Of course as you say, that's a personal opinion, and 
>>perhaps I'm
>>deluded & you'll persuade me otherwise :-) I look forward to that
>>process.
>>
>>	Thanks for listening,
>>
>>	Regards,
>>
>>		Michael.
>>
>>-- 
>> michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot
>>
>>
>>    
>>