[poppler] Patch: New additions in FileSpec

[Pino Toscano]

Hi,

Alle mercoledì 6 aprile 2011, srinivas adicherla ha scritto:
> For i/o checks for fseek() and ftell(),
> here we don't have to check for 10 bytes.

The point is: I/O can fail, and you have to check for it.

> +static GBool getFileSpecObject(XRef *xref, Stream *stream, const
> char* fileName, Object *obj)
> +{
> +  Object fileSpec, obj1, obj2;
> +  int length;
> +
> +  GooString *filename = new GooString(fileName);

Please give it a slightly different name, otherwise it is easy to 
misread filename and fileName.

> +  char *fileNameUTF16 = pdfDocEncodingToUTF16(filename, &length);
> +
> +  fileSpec.initDict(xref);
> +  fileSpec.dictSet("Type", obj1.initName("Filespec"));
> +  fileSpec.dictSet("F", obj1.initString(filename));
> +  fileSpec.dictSet("UF", obj1.initString(new
> GooString(fileNameUTF16, length)));
> +
> +  delete []fileNameUTF16;
> +  delete(filename);

no, obj1.initString(filename) already takes ownership of filename, so 
you don't delete it.

> +GBool createFilespec (XRef *xref, GooString *filePathA, Object *obj)

"FileSpec", not "Filespec"

> +{
> [...]
> +  if (!(size = ftell(fs)))
> +    return gFalse;

no, any return value >= 0 means success, while -1 means error.
also, you miss the error() call for ftell() failure

> +  // Extract the video name from the file uri 

again video name?!

srinivas, let's be frank now: do you ever *read* your patches, or you 
just do some partially untested changes, rediff and send?

-- 
Pino Toscano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 190 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/poppler/attachments/20110406/fb1a817d/attachment.pgp>