desktop-packages team mailing list archive

[Adrian Johnson <ajohnson@xxxxxxxxxxx>]

(In reply to Adam Reichold from comment #77)
> Some minor suggestions:
> 
> * The naming of BLOCK_SIZE and block_len in hashSignedDataBlock seems
> misleading to me, maybe CHUNK_SIZE and block_len?
> 
> * The method hashSignedDataBlock could probably be replaced by a static
> function taking the stream and the handler? This should give the compiler
> more optimization possibilities than if it is visible in other translation
> units.
> 
> * I think the while loop within could become a for loop for better
> readability with the case reduced to computing the number of bytes to read
> instead of two separate calls to doGetChars and updateHash.

I don't think there is much benefit in making that function a static. It
is not something that is called a frequently from an inner loop.

I think the function would read better as "hashSignedByteRange" as "byte
range" is the terminology used in the PDF reference section in
signatures. Instead of setting the stream offset before calling this
function it would better easier to understand the code if the function
took an offset and length.

I agree that the two calls to doGetChars and updateHash should be merged
but I don't think a for loop is the best way to process loops where the
increment is not exactly the same on each iteration. Maybe something
like this:

  void FormFieldSignature::hashSignedByteRange(SignatureHandler *handler,
     Goffset start, Goffset len)
  {
    const int CHUNK_SIZE = 4096;
    unsigned char buffer[CHUNK_SIZE];
    Goffset i = 0;
    int byte_count = CHUNK_SIZE;

    doc->getBaseStream()->setPos(start);
    while (i < len)
    {
      if (i + CHUNK_SIZE > len)
        byte_count = len - i;

      doc->getBaseStream()->doGetChars(byte_count, buffer);
      handler->updateHash(buffer, byte_count);
      i += byte_count;
    }
  }

I renamed the "signed_data_buffer" to "buffer" as whenever I see
"signed" in C/C++ I think of the integer type. "unsigned char
signed_data_buffer" is confusing to read.

I would prefer the buffer be moved out to the class. It is better not to
allocate large buffers on the stack. We may later increase the buffer
size.

I don't mind if we fix all this later. It doesn't have to hold up the
initial release.

-- 
You received this bug notification because you are a member of Desktop
Packages, which is subscribed to evince in Ubuntu.
https://bugs.launchpad.net/bugs/1085526

Title:
  ubuntu pdf doc viewer will not let me sign a document

Status in Poppler:
  Confirmed
Status in evince package in Ubuntu:
  Triaged

Bug description:
  Just updated in last few weeks, i think ubuntu 12.4

  To sign the document i have to send it to my neighbors windows
  computer, open it, sign it, then send it, then I get a note from echo
  sign that the document was sent with my signature.

  ProblemType: Bug
  DistroRelease: Ubuntu 11.10
  Package: evince 3.2.1-0ubuntu2.3
  ProcVersionSignature: Ubuntu 3.0.0-27.44-generic 3.0.45
  Uname: Linux 3.0.0-27-generic i686
  ApportVersion: 1.23-0ubuntu4
  Architecture: i386
  Date: Fri Nov 30 18:13:25 2012
  ExecutablePath: /usr/bin/evince
  InstallationMedia: Ubuntu 11.04 "Natty Narwhal" - Release i386 (20110427.1)
  ProcEnviron:
   PATH=(custom, no user)
   LANG=en_US.UTF-8
   SHELL=/bin/bash
  SourcePackage: evince
  UpgradeStatus: Upgraded to oneiric on 2012-11-18 (12 days ago)

To manage notifications about this bug go to:
https://bugs.launchpad.net/poppler/+bug/1085526/+subscriptions