Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Mac updates #384

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

Mac updates #384

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
d9ae6df
Get macOS working again.
ahknight Apr 4, 2017
1aea1d4
Get xattrs working on macOS.
ahknight Apr 4, 2017
39b7772
Fix issue with AT_FDCWD
ahknight Apr 4, 2017
d1aec14
Updating return/variable types
ahknight Apr 5, 2017
6cfd4d5
Clean up some cpp logic for __APPLE__
ahknight Apr 5, 2017
59ece5f
Adding position arg to xattr APIs
ahknight Apr 5, 2017
3b0d371
abstract futimesat and getting of atime/mtime
trapexit Apr 6, 2017
bca8404
Fix some compile errors for Travis
ahknight Apr 6, 2017
b0fb518
restructure osx fallocate
trapexit Apr 6, 2017
a00dc9b
Merge branch 'mac-updates' of http://github.com/ahknight/mergerfs int…
trapexit Apr 6, 2017
6a67a92
tweak types
trapexit Apr 6, 2017
3b2fa95
Silencing an analyzer notice (strlcat returns the size of the string …
ahknight Apr 6, 2017
25fe57f
Removed stray xattr flag
ahknight Apr 6, 2017
d0fa9ef
handle 32bit inode sizes
trapexit Apr 6, 2017
cf567f1
remove unnecessary futimesat abstraction
trapexit Apr 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/fs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@ namespace fs
return -1;

dev = st.st_dev;
for(int i = 0, ei = srcmounts.size(); i != ei; i++)
for(size_t i = 0, ei = srcmounts.size(); i != ei; i++)
{
fs::path::make(&srcmounts[i],fusepath,fullpath);

Expand Down Expand Up @@ -241,7 +241,7 @@ namespace fs
if(spaceavail <= mfs)
continue;

mfs = spaceavail;
mfs = (fsblkcnt_t)spaceavail;
mfsbasepath = &basepaths[i];
}

Expand Down
2 changes: 1 addition & 1 deletion src/fs_acl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ namespace fs
bool
dir_has_defaults(const std::string &fullpath)
{
int rv;
ssize_t rv;
std::string dirpath = fullpath;

fs::path::dirname(dirpath);
Expand Down
2 changes: 1 addition & 1 deletion src/fs_base_getxattr.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace fs
{
static
inline
int
ssize_t
lgetxattr(const std::string &path,
const char *attrname,
void *value,
Expand Down
6 changes: 3 additions & 3 deletions src/fs_base_ioctl.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ namespace fs
static
inline
int
ioctl(const int fd,
const int request,
void *data)
ioctl(const int fd,
const unsigned long request,
void *data)
{
return ::ioctl(fd,request,data);
}
Expand Down
2 changes: 1 addition & 1 deletion src/fs_base_listxattr.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ namespace fs
{
static
inline
int
ssize_t
llistxattr(const std::string &path,
char *list,
const size_t size)
Expand Down
2 changes: 1 addition & 1 deletion src/fs_base_readlink.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace fs
{
static
inline
int
ssize_t
readlink(const std::string &path,
char *buf,
const size_t bufsiz)
Expand Down
45 changes: 23 additions & 22 deletions src/fs_clonefile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@
using std::string;
using std::vector;

int
ssize_t
writen(const int fd,
const char *buf,
const size_t count)
{
size_t nleft;
ssize_t nwritten;
ssize_t nwritten = 0;

nleft = count;
while(nleft > 0)
Expand All @@ -67,24 +67,24 @@ writen(const int fd,
return -1;
}

nleft -= nwritten;
buf += nwritten;
nleft -= (size_t)nwritten;
buf += (size_t)nwritten;
}

return count;
return nwritten;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This shouldn't make a difference so long as count > 0 and it's a successful write/s but I see now that if count == 0 then it won't behave correctly even with your change. I'll need to fix that.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It shouldn't, no. But while making hfsinspect I ran into this issue a few times. There was some kind of situation where the read returned early but I wasn't at the end of the file/device yet and the next read kept going. So, since then, I've just always returned the real count so that client apps' counters are accurate and can handle that.

Copy link
Owner

@trapexit trapexit Apr 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get that for the outer function (copyfile_rw) but for writen the only possible bug should be that it doesn't accept zero for byte length. writen should not return till all bytes passed in are written or an error.

I think the issue is actually in copyfile_rw in which a zero return from read isn't handled properly. Should exit the loop. This is a generic bug. I'll probably fix that separately.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think of it as a data pipe. The OS gives us a total of the bytes written and then we hand that back to FUSE to deliver to the application. If anything goes wrong along the way then returning a fixed value might lead to odd things, especially for a partial write (it's not a fail because data changed, but it's not a success because the whole request wasn't completed). That'd be my only concern.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function has nothing to do with FUSE. It's for copying data between to underlying drives.

A write, according to standards, does not return 0 unless it's asked to write 0. It can return from 1 to count or -1. That's why writen exists. To make sure that the whole buffer is written.

The problem is the outer function. It should exit on EOF but write should never return anything but 1 - bufsize or -1. And writen should manage being passed a zero bufsize.

}

static
int
ssize_t
copyfile_rw(const int fdin,
const int fdout,
const size_t count,
const size_t blocksize)
{
ssize_t nr;
ssize_t nw;
ssize_t bufsize;
size_t totalwritten;
size_t bufsize;
ssize_t totalwritten;
vector<char> buf;

bufsize = (blocksize * 16);
Expand All @@ -93,7 +93,7 @@ copyfile_rw(const int fdin,
fs::lseek(fdin,0,SEEK_SET);

totalwritten = 0;
while(totalwritten < count)
while(totalwritten < (ssize_t)count)
{
nr = fs::read(fdin,&buf[0],bufsize);
if(nr == -1)
Expand All @@ -103,29 +103,30 @@ copyfile_rw(const int fdin,
return -1;
}

nw = writen(fdout,&buf[0],nr);
nw = writen(fdout,&buf[0],(size_t)nr);
if(nw == -1)
return -1;

totalwritten += nw;
}

return count;
return totalwritten;
}

static
int
ssize_t
copydata(const int fdin,
const int fdout,
const size_t count,
const size_t blocksize)
{
int rv;
ssize_t rv;
off_t scount = (off_t)count;

fs::fadvise(fdin,0,scount,POSIX_FADV_WILLNEED);
fs::fadvise(fdin,0,scount,POSIX_FADV_SEQUENTIAL);

fs::fadvise(fdin,0,count,POSIX_FADV_WILLNEED);
fs::fadvise(fdin,0,count,POSIX_FADV_SEQUENTIAL);

fs::fallocate(fdout,0,0,count);
fs::fallocate(fdout,0,0,scount);

rv = fs::sendfile(fdin,fdout,count);
if((rv == -1) && ((errno == EINVAL) || (errno == ENOSYS)))
Expand Down Expand Up @@ -153,18 +154,18 @@ ignorable_error(const int err)

namespace fs
{
int
ssize_t
clonefile(const int fdin,
const int fdout)
{
int rv;
ssize_t rv;
struct stat stin;

rv = fs::fstat(fdin,stin);
if(rv == -1)
return -1;

rv = ::copydata(fdin,fdout,stin.st_size,stin.st_blksize);
rv = ::copydata(fdin,fdout,(size_t)stin.st_size,(size_t)stin.st_blksize);
if(rv == -1)
return -1;

Expand All @@ -191,11 +192,11 @@ namespace fs
return 0;
}

int
ssize_t
clonefile(const string &in,
const string &out)
{
int rv;
ssize_t rv;
int fdin;
int fdout;
int error;
Expand Down
6 changes: 5 additions & 1 deletion src/fs_inode.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ namespace fs
void
recompute(struct stat &st)
{
uint64_t st_dev = st.st_dev; /* Mac OS has a 32-bit device ID */
/*
Some OSes have 32-bit device IDs, so box this up first.
This does also presume a 64-bit inode value.
*/
uint64_t st_dev = (uint64_t)st.st_dev;
st.st_ino |= (st_dev << 32);
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/fs_movefile.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ namespace fs
return -1;

fdin_st.st_size += additional_size;
rv = fs::mfs(basepaths,fdin_st.st_size,fdout_path);
rv = fs::mfs(basepaths,(uint64_t)fdin_st.st_size,fdout_path);
if(rv == -1)
return -1;

Expand Down
Loading