[Libguestfs] [PATCH 1/3] New API: case-sensitive-path to return case sensitive path on NTFS 3g fs

[Richard W.M. Jones]

On Mon, Oct 26, 2009 at 11:03:04AM +0000, Matthew Booth wrote:
> On 26/10/09 09:20, Richard W.M. Jones wrote:
>> +char *
>> +do_case_sensitive_path (const char *path)
>> +{
>> +  char ret[PATH_MAX+1] = "/";
>
> Is PATH_MAX on Windows <= POSIX PATH_MAX? Does ntfs_3g honour this?  
> Seems like a grey area. It would be safer to realloc this buffer as  
> necessary. You also wouldn't need the strdup() at the end.

PATH_MAX is a Linux thing.  NTFS 3g couldn't export something through
the Linux VFS unless it honoured this.

>> +  size_t next = 1;
>> +
>> +  /* MUST chdir ("/") before leaving this function. */
>> +  if (chdir (sysroot) == -1) {
>> +    reply_with_perror ("%s", sysroot);
>> +    return NULL;
>> +  }
>
> I'm not convinced chdir is necessary in this function if you use  
> openat() throughout.

I'm pretty sure I need opendirat to make this work, and that function
doesn't seem to exist (checked on Fedora 11).

>> +  /* First character is a '/'.  Take each subsequent path element
>> +   * and follow it.
>> +   */
>
> A check that *path == '/' wouldn't go amiss here. I'd stick an assert  
> in, but then DV might shoot me ;)

This is already checked in the generated code.

>> +      reply_with_error ("case_sensitive_path: path contained . or .. elements");
>
> This string isn't internationalised. I won't mention all of these.

We don't internationalize any string in the daemon (yet).

>> +  ("case_sensitive_path", (RString "rpath", [Pathname "path"]), 197, [],
>> +   [InitISOFS, Always, TestOutput (
>> +      [["case_sensitive_path"; "/DIRECTORY"]], "/directory");
>> +    InitISOFS, Always, TestOutput (
>> +      [["case_sensitive_path"; "/Known-1"]], "/known-1");
>> +    InitBasicFS, Always, TestOutput (
>> +      [["mkdir"; "/a"];
>> +       ["mkdir"; "/a/bbb"];
>> +       ["touch"; "/a/bbb/c"];
>> +       ["case_sensitive_path"; "/A/bbB/C"]], "/a/bbb/c")],
>
> These don't exercise the following cases you've coded for:
>
> /
> /foo//bar
> /foo/../bar
> /foo///bar/

Agreed that more tests are needed here.  I will add them in the next
version.

> This API seems like an unfortunate pimple. Would it not be better to  
> *not* export this API, and instead call this automatically everywhere  
> the daemon opens a file on ntfs?

But how does it know what the underlying filesystem is up to?  I agree
it's a bug in NTFS 3g, but changing all the code to do very expensive
magic doesn't seem like the right way, and you can't detect which
filesystems are broken like this.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
New in Fedora 11: Fedora Windows cross-compiler. Compile Windows
programs, test, and build Windows installers. Over 70 libraries supprt'd
http://fedoraproject.org/wiki/MinGW http://www.annexia.org/fedora_mingw