Opened 13 years ago

Closed 12 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 13 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 12 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 12 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 12 years ago by bird (previous) (diff)

comment:4 Changed 12 years ago by bird

Milestone: libc-0.6.5
Status: newaccepted

comment:5 Changed 12 years ago by bird

Summary: _fullpath() changes current drivelibc: _fullpath() changes current drive

comment:6 Changed 12 years ago by bird

Status: acceptedassigned

comment:7 Changed 12 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 12 years ago by bird

Resolution: fixed
Status: assignedclosed

r3773 fixes _dt_split.

Case hopefully closed.

Note: See TracTickets for help on using tickets.