Hi there.
On 07/02/2010 01:38 AM, srinivas adicherla wrote:
> +static void
> +get_id(char* encodedid,char *pdfid) {
> +
> sprintf(pdfid,"%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x",
> encodedid[0] & 0xff,encodedid[1] & 0xff,
It would be nice if you put spaces after commas. Also, if you wrapped the line
before the arguments that get sprintf'ed, the lines would fit in fewer columns.
> + char tmpid[33];
Why not give a name to magic constants?
> typedef struct _PopplerDocument PopplerDocument;
> +typedef struct _PopplerDocumentId PopplerDocumentId;
> typedef struct _PopplerIndexIter PopplerIndexIter;
(...)
> PROP_PERMISSIONS,
> - PROP_METADATA
> + PROP_METADATA,
> + PROP_PERMANENT_ID,
> + PROP_UPDATE_ID
> };
There seems to be some kind of consistency problems between your indentation
style and that of poppler. While I would favor something like the Linux kernel
coding conventions, I would stick to a project's coding conventions for
contributed code.
> }
> }
> break;
> + case PROP_PERMANENT_ID:
> + {
> + GooString permanent_id,update_id;
> + if (document->doc->getID(&permanent_id,&update_id))
> + {
> + g_value_set_string(value,permanent_id.getCString());
> + }
> + break;
> + }
The indentation here is messy. Also, is weird to have the case body inside
braces. As C++ allows you to mix declarations and statements, the braces are not
necessary for the declaration of the variables (which you are, BTW, not
separating with a comma).
> + case PROP_UPDATE_ID:
> + {
> + GooString permanent_id,update_id;
> + if (document->doc->getID(&permanent_id,&update_id))
> + {
> + g_value_set_string(value,update_id.getCString());
> + }
> + break;
> + }
Now, something of a higher level: why not opt to use some functions
getPermanentID and getUpdateID, and implement getID as a wrapper around them?
> + g_object_class_install_property
> + (G_OBJECT_CLASS (klass),
> + PROP_PERMANENT_ID,
> + g_param_spec_string ("permanent-id",
Any reason for the space before an open parenthesis?
> +struct _PopplerDocumentId
> +{
> + gchar permanent_id[33];
> + gchar update_id[33];
> +};
Some magic constants again.
Regards.
_______________________________________________
poppler mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/poppler
