pdf-devel
[Top][All Lists]
Advanced

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

Re: [pdf-devel] [PATCH] token reader unit test update


From: Michael Gold
Subject: Re: [pdf-devel] [PATCH] token reader unit test update
Date: Thu, 29 Oct 2009 17:21:03 -0400
User-agent: Mutt/1.5.20 (2009-06-14)

On Tue, Oct 27, 2009 at 11:36:37 -0700, Pierre Filot wrote:
> Hi,
> 
> -I added few changes to pdf-token-reader.c regarding comments token.

Your change has flush_token return PDF_EBADFILE if no EOL character was
seen, but I don't believe this change is needed.  There are only 2 times
we'd call flush_token (via exit_state) from the COMMENT state:
 - when an EOL character is seen, in handle_char
 - at the end of the pdf_stm_t ("EOF"), in pdf_token_read

PDF32000 doesn't say that an EOL marker is required to end a comment,
although it does say in 7.5.1
  "Each line shall be terminated by an end-of-line (EOL) marker."

I don't think it would be useful to reject a file for violating this
rule at EOF; but if we decide to do that, we should do it in a more
general way since the rule isn't specific to any token type.

> Otherwise, the main changes are affecting the token-reader unit test.

I'll try to take a closer look at these changes soon.

For now, please avoid trailing whitespace, don't use tabs (except in the
ChangeLog), and limit lines to 80 characters.  File timestamp comments
just need the username of the last editor, so you can remove "mgold - ".

...
> -About streams, in pdf-token-reader.c we allow the keyword stream to
> be followed by white-space and comments, but I can't find that in the
> specification (PDF Reference , version 1.7)
>...
> So why don't we limit the character following stream to an eol. 

I don't remember why I did that, but I'm fine with this change.  It
would require a new substate to track whether we've already seen a CR
(like string substate 1).

> -In the unit test pedf-token-read.c, in pdf_token_read_eos is written:
> /*TODO: verify stream position */ what should I do exactly ? 

Verify that pdf_stm_tell returns the expected position.

-- Michael

Attachment: signature.asc
Description: Digital signature


reply via email to

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