the use of plfs_getxattr relies upon the user knowing the length of the extended attribute

[dshrader]

Please see Issue #331 for where this behavior was noticed. In short, we are not following the capabilities of POSIX getxattr in that a user can find out how big the xattr is so that they can make sure they have enough memory for it. Right now, we rely on the user to provide the length of the attribute, which is somewhat naive. If the length is too small, we return a truncated value; if the length is too big, we read off the end of the file that houses the xattr (generates an error).

[dshrader]

Consensus from comments in 331 suggest adding another argument to plfs_getxattr would be appropriate. Additional error checking should occur to make sure what the user passes as the length is big enough for the xattr. It doesn't look like we have any code that can determine the size of an already set xattr, so this would have to be developed. Probably in XAttr.cpp...

[chuckcranor]

On Thu, Dec 12, 2013 at 09:47:32AM -0800, David wrote:

Consensus from comments in 331 suggest adding another argument to
plfs_getxattr would be appropriate. Additional error checking should
occur to make sure what the user passes as the length is big enough
for the xattr. It doesn't look like we have any code that can determine
the size of an already set xattr, so this would have to be developed.
Probably in XAttr.cpp...

prob just IOStore Stat() the key file...? st_size of the file would
be the size of the value.

struct stat st;
full_path = path + "/" + key;
err = this->canback->store->Stat( full_path.c_str(), &st);

chuck

[johnbent]

Also add length as a parameter to setattr. getattr can discover length by stat'ing the file, no?

On Dec 12, 2013, at 10:47 AM, David notifications@github.com wrote:

Consensus from comments in 331 suggest adding another argument to plfs_getxattr would be appropriate. Additional error checking should occur to make sure what the user passes as the length is big enough for the xattr. It doesn't look like we have any code that can determine the size of an already set xattr, so this would have to be developed. Probably in XAttr.cpp...

—
Reply to this email directly or view it on GitHub.

[brettkettering]

The client user should definitely not stat the file, as that's assuming a certain implementation and the implementation can change. It's safest to have a library call that should know the implementation get the value.

Brett

-----Original Message-----
From: John Bent [notifications@github.commailto:notifications@github.com]
Sent: Thursday, December 12, 2013 11:52 AM Mountain Standard Time
To: plfs/plfs-core
Subject: Re: [plfs-core] the use of plfs_getxattr relies upon the user knowing the length of the extended attribute (#335)

Also add length as a parameter to setattr. getattr can discover length by stat'ing the file, no?

On Dec 12, 2013, at 10:47 AM, David notifications@github.com wrote:

Consensus from comments in 331 suggest adding another argument to plfs_getxattr would be appropriate. Additional error checking should occur to make sure what the user passes as the length is big enough for the xattr. It doesn't look like we have any code that can determine the size of an already set xattr, so this would have to be developed. Probably in XAttr.cpp...

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/335#issuecomment-30450192.

[dshrader]

plfs_setxattr already has a required length parameter, so I don't think it needs to change:

plfs_setxattr(Plfs_fd *fd, const void *value, const char *key, size_t len)

It looks like we do follow POSIX's setxattr syntax as the return value for setxattr is 0 for success, -1 for failure, which is basically what our plfs_errno does. So I think we're ok here unless I have missed something.

[johnbent]

The client user would not be doing the stat. The stat would be done
internally within the plfs code. Chuck posted a similar comment above
where he explained it better than I did.

On Thu, Dec 12, 2013 at 12:47 PM, Brett Kettering
notifications@github.comwrote:

The client user should definitely not stat the file, as that's assuming a
certain implementation and the implementation can change. It's safest to
have a library call that should know the implementation get the value.

Brett

-----Original Message-----
From: John Bent [notifications@github.commailto:notifications@github.com]

Sent: Thursday, December 12, 2013 11:52 AM Mountain Standard Time
To: plfs/plfs-core
Subject: Re: [plfs-core] the use of plfs_getxattr relies upon the user
knowing the length of the extended attribute (#335)

Also add length as a parameter to setattr. getattr can discover length by
stat'ing the file, no?

On Dec 12, 2013, at 10:47 AM, David notifications@github.com wrote:

Consensus from comments in 331 suggest adding another argument to
plfs_getxattr would be appropriate. Additional error checking should occur
to make sure what the user passes as the length is big enough for the
xattr. It doesn't look like we have any code that can determine the size of
an already set xattr, so this would have to be developed. Probably in
XAttr.cpp...

—
Reply to this email directly or view it on GitHub.

—
Reply to this email directly or view it on GitHub<
https://github.com/plfs/plfs-core/issues/335#issuecomment-30450192>.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/335#issuecomment-30455126
.

[dshrader]

We should probably target this fix to go in to 2.6 since it will change the external API.