[Desktop_printing] CUPS code quality ...

michael meeks michael.meeks at novell.com
Thu Feb 2 03:02:13 PST 2006


Hi there,

	So - Till asked me to write something public about this.

	This is based on a rumor I heard that there was some plan to
"standardize on the CUPS API".

	My experience of using the CUPS API is that it is badly designed,
painful to use, synchronous (and likes to take an unpredictable time to
do any given operation: ms -> minutes) and to top the lot - thread
unsafe by design.

	So - of course, we try to push CUPS work into thread(s) anyway since we
can't have the 'get-list-of-printers' call randomly blocking OO.o for
ages on random machines, with random N/W configurations. [ we're slow
enough in general anyway ;-]

	Let me give one example eg. the OO.o psprint/ code does:

OString CUPSWrapper::cupsGetPPD( const char* pPrinter )
{
    OString aResult;
    m_aGetPPDMutex.acquire();
**  // if one thread hangs in cupsGetPPD already, don't start another
    if( ! m_bPPDThreadRunning )
    {
        m_bPPDThreadRunning = true;
        GetPPDAttribs* pAttribs = new GetPPDAttribs( m_pcupsGetPPD,
                                                     pPrinter,
                                                     &m_bPPDThreadRunning,
                                                     &m_aGetPPDMutex );
..

	So the corresponding helper 'cupsGetPPD' method that we use is:

http://svn.easysw.com/public/cups/branches/branch-1.1/cups/util.c

/*
 * 'cupsGetPPD()' - Get the PPD file for a printer.
 */
const char *				/* O - Filename for PPD file */
cupsGetPPD(const char *name)		/* I - Printer name */
{
..
}

	Please note this method returns a pointer to an internal static buffer:

const char *				/* O - Filename for PPD file */
cupsGetPPD2(http_t     *http,		/* I - HTTP connection */
            const char *name)		/* I - Printer name */
{
  int		i;			/* Looping var */
  http_t	*http2;			/* Alternate HTTP connection */
  ipp_t		*request,		/* IPP request */
		*response;		/* IPP response */
  ipp_attribute_t *attr;		/* Current attribute */
  cups_lang_t	*language;		/* Local language */
  int		fd;			/* PPD file */
  char		uri[HTTP_MAX_URI],	/* Printer URI */
		printer[HTTP_MAX_URI],	/* Printer name */
		method[HTTP_MAX_URI],	/* Method/scheme name */
		username[HTTP_MAX_URI],	/* Username:password */
		hostname[HTTP_MAX_URI],	/* Hostname */
		resource[HTTP_MAX_URI];	/* Resource name */
  int		port;			/* Port number */
  http_status_t	status;			/* HTTP status from server */
  static char	filename[HTTP_MAX_URI];	/* Local filename */
..
 /*
  * Return the PPD file...
  */
  return (filename);
}

	So if even 1/2 the brain-power that was consumed adding the comment:
"/* Looping var */" to an 'int i' statement was used researching the
problems & failures of API design still biting people long afterwards.
And/or the relevant costs speed of dynamic memory allocation vs. socket
IPC - this API wouldn't be as broken as it is. [ also note the extensive
use of static size buffers etc. ;-]

	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.

	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.

	Please don't standardize on it ;-) wrap it, re-write the API from
scratch [ under eclectic copyright ? ;-] do whatever; but - lets get
some people expert in API design to put some thought & review into the
design of this critical API, and lets ensure it's thread safe (or at
least can be made thread-safe later :-).

	Thanks,

		Michael. [ who now expects to be soundly flamed ]

-- 
 michael.meeks at novell.com  <><, Pseudo Engineer, itinerant idiot




More information about the Printing-summit mailing list