pdf-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [pdf-devel] Token writer code


From: jemarch
Subject: Re: [pdf-devel] Token writer code
Date: Wed, 28 Oct 2009 21:24:58 +0100
User-agent: Wanderlust/2.14.0 (Africa) SEMI/1.14.6 (Maruoka) FLIM/1.14.8 (Shijō) APEL/10.6 Emacs/23.0.60 (i686-pc-linux-gnu) MULE/6.0 (HANACHIRUSATO)

Hi Michael.

   Here's the current version of my token writer code for review.  It's
   basically complete, but I'll need to read over it again and do some more
   testing before submitting a final version.

Many thanks.  My comments follow.

   +  /* PDF32000 7.5.1: "lines that are not part of stream object data
   +   * are limited to no more than 255 characters"... */
   +  new_tokw->max_line_length = 255;  /* set to 0 to disable */
   +
   +  new_tokw->buffer = pdf_buffer_new (32768);

Please use preprocessor constants for those values, and document them
in the architecture guide.

   +pdf_status_t
   +pdf_token_writer_destroy (pdf_token_writer_t writer)
   +{
   +  if (!writer) return PDF_EBADDATA;

If you change the API definition of functions documented in
gnupdf.texi, please include the proper modifications in your patch
(like in this case, where you introduce a new possible return value
for 'pdf_token_writer_destroy').


   +/***** Numeric tokens *****/
   +
   +/* Encode snprintf output for PDF.  'len' is the return value of
   snprint.

Typo: snprintf.

   +        case READER_FLAGS_ARG:
   +          {
   +            reader_flags = atoi(optarg);
   +            break;
   +          }
   +        case WRITER_FLAGS_ARG:
   +          {
   +            writer_flags = atoi(optarg);
   +            break;

Please don't use ato* functions.  Use the strto* functions instead:
they provide proper error checking.

Despite those minor issues I like what you wrote.

-- 
Jose E. Marchesi  <address@hidden>
                  http://www.jemarch.net
GNU Project       http://www.gnu.org




reply via email to

[Prev in Thread] Current Thread [Next in Thread]