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

[jemarch]

Hi Zac.

Many thanks for fixing the patch.

   +  if (path_name == NULL)
   +    {
   +      return PDF_FALSE;
   +    }

Maybe would be a good idea to write system-dependent path syntax
checking functions. Something like 'pdf_fsys_posix_path_name_p' and
'pdf_fsys_w32_path_name_p'. What do you think?

   +  ret_code = pdf_text_get_host (&ascii_path, &ascii_path_len, path_name, 
pdf_text_get_host_encoding());

Note that there is a static function named
'pdf_fsys_disk_get_host_path'. Would be better to use it instead of
'pdf_text_get_host' directly.

There may be a problem with our approach of getting the host-encoded
path name and using it with the OS functions. The filenames In GNU and
POSIX systems are encoded using either ASCII, ISO-8859-X or
filesystem-safe Unicode encodings such as UTF-32, UTF-16 or
UTF-8. File names using those encodings never contains null octects
and can be safely used with libc functions expecting null-terminated
strings.

My concern is: are we in the same case in Windoze machines? Could
'pdf_text_get_host' return a string encoded in some filesystem-unsafe
encoding such as UCS-2? We should investigate how windows manages the
encoding of file names. Any idea?

   +#ifdef PDF_HOST_WIN32
   +  hFile = FindFirstFile (ascii_path, &data);
   +  if (hFile == NULL)
   +    goto error_cleanup;
   +  else
   +    FindClose (hFile);
   +#else

How do 'FindFirstFile' manages relative paths? What is the "default"
volume used if the path does not contain a volume specification? Do it
support "device" filenames such as PRN?

Regarding the device filenames, note that there is a function in
'pdf-fsys-disk.c' called 'pdf_fsys_disk_win32_device_p' that will tell
you if a given path names a device. My suggestion is to directly
return PDF_TRUE if the path is a device (beware: that function was
never tested).

   +  if (file == NULL)
   +    goto error_cleanup;
   +  else
   +    fclose (file);

Please use braces {} in any if or else body even if it only contain a
single statement, such as in:

   if (file == NULL)
   {
     goto error_cleanup;
   }
   else
   {
     fclose (file);
   }