<html> <head> <base href="https://bugs.freedesktop.org/"> </head> <body> <div> <a class="bz_bug_link bz_status_NEW " title="NEW - Enumerate PDF named destinations" href="https://bugs.freedesktop.org/show_bug.cgi?id=97262#c31">Comment # 31</a> on <a class="bz_bug_link bz_status_NEW " title="NEW - Enumerate PDF named destinations" href="https://bugs.freedesktop.org/show_bug.cgi?id=97262">bug 97262</a> from <a class="email" href="mailto:trueroad@trueroad.jp" title="Masamichi Hosoda <trueroad@trueroad.jp>"> Masamichi Hosoda</a> <pre>(In reply to Adrian Johnson from <a href="show_bug.cgi?id=97262#c26">comment #26</a>) > I've been working on a patch for CairoOutputDev that requires access to a > list of named destinations. The core patches would be helpful. Looking at > the patches: Thank you for your reviewing. > 0001 - Looks good. Although I suggest renaming "constructLinkDest" to > "createLinkDest". I'll change the patch. > 0002, 0003 - I don't think we need separate functions for Name Trees and > Dests Dict access. I suggest have one set of functions for accessing names > eg: > > int numDests(); > GooString *getDestsName(int i); > LinkDest *getDestsLink(int i); > > Internally, the functions should combine the Name Tree dests and dict dests > into one list. Name Tree is recorded in Catalog::destNameTree. It has GooString for its name. So getDestName() can return the pointer of GooString. The return value should not be deleted. Dests Dict is recoded in Catalog::dests. However, it does not have GooString for its name. It has only char* for its name. So getDestName() have to construct GooString from char*. The return value should be deleted. Or, would you like to add a new member variable GooString *destnames to class Catalog? In this case, Catalog::getDestName() creates destnames array and Catalog::~Catalog() deletes it. The return value should not be deleted. > 0005 - I know that Jose suggested returning a GHashTable. Since the main > purpose of a hash table is mapping keys to values, and the > poppler_document_find_dest() function already does this, maybe the names > could be returned by either: > > 1) A list of just the names. The user can use poppler_document_find_dest() > to get the dests. Is is something like GList *poppler_document_build_dests_namelist() ? It returns the pointer of GList which contains GBytes for destination name. If you would like it, I'll create a patch. However, unfortunately, if the name contains \0, poppler_document_find_dest() cannot find it. > or > 2) A binary tree of names and dests. This has the advantage that iterating > the names is in alphabetical order instead of the the random order of the > hash table. I'd like this. > Also, poppler_document_get_dests() creates the hash table each time it is > called. It maybe better to cache the hash table or use a function name like > poppler_document_build_dests_hashtable() since "get" implies fast access. I'll change the patch to poppler_document_build_dests(). > + key = g_bytes_new (catalog->getDestsName (i), > + strlen (catalog->getDestsName (i))); > > You are using g_bytes because the name can contain \0 but you are using > strlen here. Also the getDestName() in Catalog will need to return a > GooString, not char *. It handles Dests Dict. The name of Dests Dict cannot contain \0. Only the name of Name Tree can contain \0. > Although it is not clear to me whether destination names can actually > contain \0. PDF32000 section 12.3.2.3 states "The keys in the name tree may > be > treated as text strings for display purposes". Almost UTF-16 strings contains \0.</pre> </div> <hr> You are receiving this mail because: <ul> <li>You are the assignee for the bug.</li> </ul> </body> </html>