Fix os::native_is_{file,dir} bug. Add tests

[unereal]

[unereal]

The example program in man page for stat(2) uses sb.st_mode & S_IFMT == xyz to check for xyz bits. Maybe this isn't what we want. /dev/log, for example, has multiple bits set:

stat.st_mode & libc::S_IFREG: True stat.st_mode & libc::S_IFMT == libc::S_IFREG: False

[lerno]

I don't understand @unereal. That can't be true unless S_IFMT doesn't contain S_IFREG then?

[unereal]

I too don't understand. This program uses both stat.st_mode & libc::S_IFMT == XYZ and stat.st_mode & XYZ

import std::io;
import libc;

// Build: c3c compile -o ~/bin/c3stat c3stat.c3

fn void c3stat(String file)
{
	Stat stat;
	if (@ok(os::native_stat(&stat, file)))
	{
		io::printn("# stat info");
		io::printfn("stat.st_mode                = %b", stat.st_mode);
		io::printfn("stat.st_mode & libc::S_IFMT = %b", stat.st_mode & libc::S_IFMT);
		io::printn("# stat.st_mode & libc::S_IFMT == XYZ");
		io::printf("file '%s' is ", file);
		switch (stat.st_mode & libc::S_IFMT)
		{
			case libc::S_IFREG:
				io::printn("regular file");
			case libc::S_IFDIR:
				io::printn("directory");
			case libc::S_IFBLK:
				io::printn("block device");
			case libc::S_IFCHR:
				io::printn("character device");
			case libc::S_IFIFO:
				io::printn("fifo");
			case libc::S_IFSOCK:
				io::printn("socket");
			case libc::S_IFLNK:
				io::printn("symbolic link");
			default:
				io::printn("unknown");
		}

		io::printn("# stat.st_mode & XYZ");
		io::printfn("file '%s' is", file);
		if (stat.st_mode & libc::S_IFREG) {
			io::printn("- regular file");
		}
		if (stat.st_mode & libc::S_IFDIR) {
			io::printn("- directory");
		}
		if (stat.st_mode & libc::S_IFBLK) {
			io::printn("- block device");
		}
		if (stat.st_mode & libc::S_IFCHR) {
			io::printn("- character device");
		}
		if (stat.st_mode & libc::S_IFIFO) {
			io::printn("- fifo");
		}
		if (stat.st_mode & libc::S_IFSOCK) {
			io::printn("- socket");
		}
		if (stat.st_mode & libc::S_IFLNK) {
			io::printn("- symbolic link");
		}
	}
	io::printn();
}

fn void main(String[] argv)
{
	io::printn("# libc constants");
	io::printfn("libc::S_IFMT                = %-16b", libc::S_IFMT);
	io::printfn("libc::S_IFREG               = %-16b", libc::S_IFREG);
	io::printfn("libc::S_IFDIR               = %-16b", libc::S_IFDIR);
	io::printfn("libc::S_IFBLK               = %-16b", libc::S_IFBLK);
	io::printfn("libc::S_IFCHR               = %-16b", libc::S_IFCHR);
	io::printfn("libc::S_IFIFO               = %-16b", libc::S_IFIFO);
	io::printfn("libc::S_IFSOCK              = %-16b", libc::S_IFSOCK);
	io::printfn("libc::S_IFLNK               = %-16b", libc::S_IFLNK);
	io::printn();

	assert(libc::S_IFMT == (libc::S_IFREG|libc::S_IFDIR|libc::S_IFBLK|libc::S_IFCHR|libc::S_IFIFO|libc::S_IFSOCK|libc::S_IFLNK));

	foreach (f : argv[1..])
	{
		c3stat(f);
	}
}

Output on debian-x64:

c3stat /dev/log /dev/tty
# libc constants
libc::S_IFMT                = 1111000000000000                
libc::S_IFREG               = 1000000000000000                
libc::S_IFDIR               =  100000000000000                
libc::S_IFBLK               =  110000000000000                
libc::S_IFCHR               =   10000000000000                
libc::S_IFIFO               =    1000000000000                
libc::S_IFSOCK              = 1100000000000000                
libc::S_IFLNK               = 1010000000000000                

# stat info
stat.st_mode                = 1100000110110110
stat.st_mode & libc::S_IFMT = 1100000000000000
# stat.st_mode & libc::S_IFMT == XYZ
file '/dev/log' is socket
# stat.st_mode & XYZ
file '/dev/log' is
- regular file
- directory
- block device
- socket
- symbolic link

# stat info
stat.st_mode                = 10000110110110
stat.st_mode & libc::S_IFMT = 10000000000000
# stat.st_mode & libc::S_IFMT == XYZ
file '/dev/tty' is character device
# stat.st_mode & XYZ
file '/dev/tty' is
- block device
- character device
- symbolic link

Using stat(1), (the correct output I suppose):

stat -c %F /dev/log /dev/tty
symbolic link
character special file

[unereal]

Okay, stat(2) follows symlinks by default. To retrieve information from the link itself, one should use lstat(2) instead.

[lerno]

Well, note that if (stat.st_mode & libc::S_IFBLK) will return true even if (stat.st_mode & libc::S_IFBLK) == libc::S_IFBLK) is false as S_IFBLK is not a mask with a single bit.

[unereal]

It turns out that stat.st_mode & libc::S_IFMT == XYZ in the original patch was correct.

[lerno]

Shouldn't it properly be (stat.st_mode & libc::S_IFBLK) == libc::S_IFBLK)?

[unereal]

Yes, (stat.st_mode & libc::S_IFBLK) == libc::S_IFBLK) should work.

Since libc::S_IFMT contains libc::S_IFBLK, (stat.st_mode & libc::S_IFMT) == libc::S_IFBLK) also works. That's how the S_ISXYZ() macros are defined in both musl and glibc.

Maybe there's something on those macros that I can't immediately see.

https://git.musl-libc.org/cgit/musl/tree/include/sys/stat.h#n50 https://sourceware.org/git/?p=glibc.git;a=blob;f=io/sys/stat.h;h=3b4ba80132d0b70390e318602d165665c56c36ff;hb=HEAD#l123

[unereal]

Yes, (stat.st_mode & libc::S_IFBLK) == libc::S_IFBLK) should work.

Looks like this style doesn't work with file '/dev/log'. It's a symbolic link but '(sb.st_mode & S_IFCHR) == S_IFCHR' passes.

Anyway, there are missing parentheses in the original patch. I'll add a commit shortly.

[lerno]

Why would you need the parenthesis?

[unereal]

I was testing various combinations of S_IFXYZ bits with and without the S_IFMT mask, with and without parenthesis around a & b == c in C code. I would convert it to C3 later tonight. But I won't. By the interactions here and elsewhere It is clear to me that these contributions are unappreciated in here, I''ll leave it to other volunteers carry the torch.

[lerno]

I was testing various combinations of S_IFXYZ bits with and without the S_IFMT mask, with and without parenthesis around a & b == c in C code.

Oh, this is because C3 has different precedence on & so for C, you need (stat.st_mode & libc::S_IFBLK) == libc::S_IFBLK, but in C3 stat.st_mode & libc::S_IFBLK == libc::S_IFBLK) this is fine.

By the interactions here and elsewhere It is clear to me that these contributions are unappreciated in here, I''ll leave it to other volunteers carry the torch.

🤔 What interactions are you referring to?

[lerno]

After checking this in detail I think it's correct. I pulled it into master.