Re: [pdf-devel] [PATCH] Implement pdf_fsys_disk_item_p in base/pdf-fsys-disk.c

[jemarch]

Hello Zac.

Many many thanks for your patch, but it needs to be reworked a bit.

   +#include <pdf-text-host-encoding.h>

When a library module needs to use the functionality from other module
it is expected to include the main ("public") header for that
module and to use only the public interface to that module.

In this way we avoid to use internal and undocumented functions: they
may change without notice broking the module using it.

So in this case the fsys module should include <pdf-text.h> and use
the public API.

   +static pdf_bool_t
   +pdf_fsys_get_ascii_str (pdf_text_t path, pdf_char_t* ascii_path);

Note that, even in static functions, we are using a specific
convention: to use the prefix of the file as the prefix for function
names. So a better name for this function would be
'pdf_fsys_disk_get_ascii_str'.

I dont entirely understand the purpose of that function. It internally
uses the 'pdf_text_utf32he_to_host' function, that (I suppose, since
it is an internal function from the text module and thus undocumented)
return a host-encoded string. The host may be using a ISO-8859-1
locale, for example.

   +  ascii_path = (pdf_char_t*)pdf_alloc (path_name->size / 4);

Here you are using internal information from a opaque type,as
documented in the reference manual:

  pdf_text_t

  A Unicode string, which must be considered as an opaque type. It
  contains the data in UTF-32BE encoding, as well as any `ISO-639-1'
  country code and/or `ISO-3166-1 alpha-2' language code applied. It
  also contains an internal list of word boundaries.

Please rewrite your patch to only use the public API of the text
module. You may find that you need more functionality from the text
module to perform some tasks; in that case we will expand the public
api of the text module to fit your needs.