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.

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

Should not assume file exist at a open call #776

Closed
wangvsa opened this issue Jun 12, 2023 · 1 comment
Closed

Should not assume file exist at a open call #776

wangvsa opened this issue Jun 12, 2023 · 1 comment

Comments

@wangvsa
Copy link
Collaborator

wangvsa commented Jun 12, 2023

This is a small assumption issue.
unifyfs_fid_open function assumes the file must exist after an open call. But the file may be opened without "O_CREAT" flag; For example, some applications use the open call to test for file existence. In this case, we should not print out an error (line 564).
Fix: replace LOGERR to LOGDBG. I will create a PR to fix this.

int unifyfs_fid_open(
unifyfs_client* client,
const char* path, /* path of file to be opened */
int flags, /* flags bits as from open(2) */
mode_t mode, /* mode bits as from open(2) */
int* outfid, /* allocated local file id if open is successful */
off_t* outpos) /* initial file position if open is successful */
{
int ret = UNIFYFS_SUCCESS;
/* check that pathname is within bounds */
size_t pathlen = strlen(path) + 1;
if (pathlen > UNIFYFS_MAX_FILENAME) {
return ENAMETOOLONG;
}
int gfid = unifyfs_generate_gfid(path);
/* attempt to create file if requested */
if (flags & O_CREAT) {
/* Ensure that we're only creating a regular file. */
int type = mode & S_IFMT;
if (type & ~S_IFREG) {
LOGERR("Only regular files can be created %s S_IFMT bits %x",
path, type);
return EINVAL;
}
mode |= S_IFREG;
int exclusive = (flags & O_EXCL);
int private = (exclusive && client->use_excl_private);
int truncate = (flags & O_TRUNC);
ret = unifyfs_gfid_create(client, path,
exclusive, private, truncate, mode);
if (ret != UNIFYFS_SUCCESS) {
return ret;
}
}
/* File should exist at this point,
* update our cache with its metadata. */
ret = unifyfs_fid_fetch(client, path);
if (ret != UNIFYFS_SUCCESS) {
/* Failed to get metadata for a file that should exist.
* Perhaps it was since deleted. We could try to create
* it again and loop through these steps, but for now
* consider this situation to be an error. */
LOGERR("Failed to get metadata on existing file %s", path);
return ret;
}

@MichaelBrim
Copy link
Collaborator

@wangvsa Yes, I agree it shouldn't be treated as an error for logging purposes - LOGDBG is good. We should also change the message to remove the word "existing"

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants