Opened 9 years ago

Closed 8 years ago

#237 closed defect (fixed)

libc: _fullpath() changes current drive

Reported by: dmik Owned by: bird
Priority: normal Milestone: libc-0.6.5
Component: libc Version: 0.6
Severity: normal Keywords:
Cc:

Description

I found that _fullpath() implicitly changes the current OS/2 drive letter under some circumstances. Here is a test case:

#include <stdio.h>
#include <stdlib.h>
#include <errno.h>
#include <string.h>

#define TEST 2

#if TEST == 1
    // OK: this doesn't change current drive
    const char *path = "CONFIG.SYS";
#elif TEST == 2
    // FAIL: this changes current drive to C:
    const char *path = "C:\\CONFIG.SYS";
#elif TEST == 3
    // FAIL: this changes current drive to C: too
    const char *path = "C:\\FAKEDIR\\CONFIG.SYS";
#endif

int main ()
{
    char buf [260];

    printf ("current drive = %c\n", _getdrive());

    int rc = _fullpath (buf, path, sizeof(buf));
    if (!rc)
        printf ("_fullpath (%s) = %s\n", path, buf);
    else
        printf ("rc = %d, errno = %s\n", rc, strerror(errno));

    printf ("current drive = %c\n", _getdrive());

    return 0;
}

I find this side effect completely unexpected and error prone.

Change History (8)

comment:1 Changed 9 years ago by dmik

The actual source of the error is the current implementation of chdir(). This implementation, as opposed to both EMX docs and MSC/VAC behavior, does change the current drive together with changing the current directory. Your reason is probably compatibility with Unix but this made it incompatible with the DOS-like world... May be we should use libc_gfNoUnix to select the way chdir() behaves? I.e. direct it to _chdir_os2() if libc_gfNoUnix is TRUE. After all, there is _chdir2() in EMX which is specifically intended to change the drive, as opposed to chdir().

Anyway, what happens in fullpath.c is:

  1. At #28, you save the CWD of either the current drive (if the drive letter is missing from the src path) or the drive that is contained in the src path.
  1. At #87, you restore the CWD using chdir() and this changes the current drive if the src path contains a drive letter and this letter is different from the current drive.

The solution to this problem is to use _chdir_os2() instead of chdir() within the fullpath() implementation.

comment:2 Changed 8 years ago by dmik

Just found another negative side effect. A call to _tempnam() (which internally calls _fullpath()) changes the current drive if %TMP% points to a different drive. This is really tricky and causes unpredictable results at runtime (see http://svn.netlabs.org/odin32/ticket/63 for an example).

comment:3 Changed 8 years ago by bird

_chdir() works the way it does on Windows now, instead of the the DOS/VAC way. This is also, as you pointed out, more compatible with what UNIX programs expect.

That _fullpath() changes the current directory and/or drive is not thread safe and needs to go.

_dt_split is also using chdir and needs to be checked up.

Last edited 8 years ago by bird (previous) (diff)

comment:4 Changed 8 years ago by bird

  • Milestone set to libc-0.6.5
  • Status changed from new to accepted

comment:5 Changed 8 years ago by bird

  • Summary changed from _fullpath() changes current drive to libc: _fullpath() changes current drive

comment:6 Changed 8 years ago by bird

  • Status changed from accepted to assigned

comment:7 Changed 8 years ago by bird

Fixed in r3769 and r3772. Found and fixed a remotely related bug in _realpath and _realrealpath in r3768 wrt to the existence of the final path component.

comment:8 Changed 8 years ago by bird

  • Resolution set to fixed
  • Status changed from assigned to closed

r3773 fixes _dt_split.

Case hopefully closed.

Note: See TracTickets for help on using tickets.