New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dirname error on relative path #1017

Closed
tiehuis opened this Issue May 17, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@tiehuis
Member

tiehuis commented May 17, 2018

See bbae626

Running zig fmt on a file gives the following strace:

getrandom("\355S\331\244h\37s\202]\247@5", 12, 0) = 12
open("/7VPZpGgfc4Jdp0A1", O_WRONLY|O_CREAT|O_EXCL|O_CLOEXEC, 0666) = -1 EACCES (Permission denied)
brk(0x21e3000)                          = 0x21e3000
munmap(0x7f34c4ea7000, 278528)          = 0
munmap(0x7f34c4e41000, 417792)          = 0
close(3)                                = 0
write(2, "error: ", 7error: )                  = 7
write(2, "AccessDenied", 12AccessDenied)            = 12
write(2, "\n", 1
)                       = 1
mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0x7f34c4f16000

Presumably the directory name passed to here https://github.com/zig-lang/zig/blob/master/std/os/index.zig#L879 is truncated to nothing so we attempt to use the root.


Stack trace for reference:

bigint.zig
error: AccessDenied
/home/user/local/lib/zig/std/os/index.zig:351:33: 0x2e4deb in ??? (zig)
                posix.EACCES => return PosixOpenError.AccessDenied,
                                ^
/home/user/local/lib/zig/std/os/index.zig:338:5: 0x2ce565 in ??? (zig)
    return posixOpenC(path_with_null.ptr, flags, perm);
    ^
/home/user/local/lib/zig/std/os/file.zig:69:24: 0x307ea9 in ??? (zig)
            const fd = try os.posixOpen(allocator, path, flags, file_mode);
                       ^
/home/user/local/lib/zig/std/os/index.zig:895:25: 0x3065b3 in ??? (zig)
                else => return err,
                        ^
/home/user/local/lib/zig/std/io.zig:450:28: 0x2fa4b4 in ??? (zig)
        self.atomic_file = try os.AtomicFile.init(allocator, dest_path, os.default_file_mode);
                           ^
/home/user/src/zig/src-self-hosted/main.zig:707:21: 0x2ebcd6 in ??? (zig)
        const baf = try io.BufferedAtomicFile.create(allocator, file_path);
                    ^
/home/user/src/zig/src-self-hosted/main.zig:86:13: 0x2e7551 in ??? (zig)
            try command.exec(allocator, args[2..]);

@tiehuis tiehuis added the bug label May 17, 2018

tiehuis added a commit that referenced this issue May 17, 2018

@tiehuis tiehuis changed the title from BufferedAtomicFile error to dirname error on relative path May 17, 2018

@andrewrk andrewrk added this to the 0.3.0 milestone May 28, 2018

@andrewrk

This comment has been minimized.

Member

andrewrk commented May 29, 2018

It just occurred to me that this happens when you do zig fmt foo.zig - and I didn't realize how to trigger the bug because this whole time I have been doing zig fmt ../foo.zig

@andrewrk

This comment has been minimized.

Member

andrewrk commented May 29, 2018

Oh, and that's what you meant by "relative path". 🤦‍♂️ sorry about that

@andrewrk andrewrk closed this in d172e3f May 29, 2018

@tiehuis

This comment has been minimized.

Member

tiehuis commented May 30, 2018

I was working around this before by just doing zig fmt ./foo.zig, so it wasn't too bad.

Is there are particular reason why we don't conform to the standard posix behaviour of dirname? (and basename). For example (from man 3 basename):

              path       dirname   basename
              /usr/lib   /usr      lib
              /usr/      /         usr
              usr        .         usr
              /          /         /
              .          .         .
              ..         .         ..

I was briefly investigating this but wasn't sure on how to apply this to all the cases of Windows on a first glance.

@andrewrk

This comment has been minimized.

Member

andrewrk commented May 30, 2018

Is there are particular reason why we don't conform to the standard posix behaviour of dirname? (and basename). For example (from man 3 basename):

It's so that we can have the fn prototype be fn(path: []const u8) []const u8 - for most cases we can simply return a smaller slice, but for this case we would not be able to do that and would have to accept an allocator argument or use some other strategy.

I thought about it more though, and I think there's an even better solution:

fn(path: []const u8) ?[]const u8

so you either get back the same slice but smaller, or null if it's the current working directory. Most callers would do os.path.dirname(path) ?? ".", or handle the null value more intelligently to avoid having unnecessary ./ in their paths.

I'll re-open this issue until that change is made.

@andrewrk andrewrk reopened this May 30, 2018

@andrewrk andrewrk closed this in 6943cef Jun 14, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment