Message ID | 1380654945-17787-1-git-send-email-sgw@linux.intel.com |
---|---|
State | Accepted |
Commit | 3a4b0e7973bef43f16058137e64600e2f890b117 |
Headers | show |
diff --git a/meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c b/meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c index 5d2c45b..53700c6 100644 --- a/meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +++ b/meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c @@ -274,8 +274,20 @@ static void add_new_file(char *name, char *path, unsigned long uid, static void add_new_fifo(char *name, char *path, unsigned long uid, unsigned long gid, unsigned long mode) { - if (mknod(path, mode, 0)) - error_msg_and_die("%s: file can not be created with mknod!", path); + int status; + struct stat sb; + + memset(&sb, 0, sizeof(struct stat)); + status = stat(path, &sb); + + + /* Update the mode if we exist and are a fifo already */ + if (status >= 0 && S_ISFIFO(sb.st_mode)) { + chmod(path, mode); + } else { + if (mknod(path, mode, 0)) + error_msg_and_die("%s: file can not be created with mknod!", path); + } chown(path, uid, gid); // printf("File: %s %s UID: %ld GID: %ld MODE: %04lo\n", // path, name, gid, uid, mode);
On Tue, 2013-10-01 at 12:15 -0700, Saul Wold wrote: > + int status; > + struct stat sb; > + > + memset(&sb, 0, sizeof(struct stat)); > + status = stat(path, &sb); Don't you want lstat() there? Also, I think *stat() is guaranteed to fill in all of sb if it returns anything other than an error, so the memset() may be redundant. I sort of wonder whether just unlink()ing the destination prior to calling mknod would be a simpler and more robust way of fixing this problem. Also, on a tangential note, you seemed to have rather a surfeit of signed-off-by lines in your email. p.
On 10/01/2013 12:49 PM, Phil Blundell wrote: > On Tue, 2013-10-01 at 12:15 -0700, Saul Wold wrote: >> + int status; >> + struct stat sb; >> + >> + memset(&sb, 0, sizeof(struct stat)); >> + status = stat(path, &sb); > > Don't you want lstat() there? Also, I think *stat() is guaranteed to > fill in all of sb if it returns anything other than an error, so the > memset() may be redundant. > I was keeping the same code style from the file function in the same code. I chose to use stat() to maintain the same failure and error handling we have currently. > I sort of wonder whether just unlink()ing the destination prior to > calling mknod would be a simpler and more robust way of fixing this > problem. > I was attempting to get a point fix for the release, we can worry about a more robust handling of the error / upgrade case in 1.5.1 or 1.6. > Also, on a tangential note, you seemed to have rather a surfeit of > signed-off-by lines in your email. > Fixed on the branch! Sau! > p. > > > >
On Tue, 2013-10-01 at 12:58 -0700, Saul Wold wrote: > On 10/01/2013 12:49 PM, Phil Blundell wrote: > > On Tue, 2013-10-01 at 12:15 -0700, Saul Wold wrote: > >> + int status; > >> + struct stat sb; > >> + > >> + memset(&sb, 0, sizeof(struct stat)); > >> + status = stat(path, &sb); > > > > Don't you want lstat() there? Also, I think *stat() is guaranteed to > > fill in all of sb if it returns anything other than an error, so the > > memset() may be redundant. > > > I was keeping the same code style from the file function in the same code. > > I chose to use stat() to maintain the same failure and error handling we > have currently. I'm not quite sure I understand the last sentence you wrote above. Can you expand on why stat() rather than lstat() is the right thing? >I was attempting to get a point fix for the release, we can worry >about a more robust handling of the error / upgrade case in 1.5.1 >or 1.6. Righto. I am blissfully ignorant of the criteria for the release so I am happy to defer to your judgement on that. p.
On 10/01/2013 01:07 PM, Phil Blundell wrote: > On Tue, 2013-10-01 at 12:58 -0700, Saul Wold wrote: >> On 10/01/2013 12:49 PM, Phil Blundell wrote: >>> On Tue, 2013-10-01 at 12:15 -0700, Saul Wold wrote: >>>> + int status; >>>> + struct stat sb; >>>> + >>>> + memset(&sb, 0, sizeof(struct stat)); >>>> + status = stat(path, &sb); >>> >>> Don't you want lstat() there? Also, I think *stat() is guaranteed to >>> fill in all of sb if it returns anything other than an error, so the >>> memset() may be redundant. >>> >> I was keeping the same code style from the file function in the same code. >> >> I chose to use stat() to maintain the same failure and error handling we >> have currently. > > I'm not quite sure I understand the last sentence you wrote above. Can > you expand on why stat() rather than lstat() is the right thing? > To be able to handle the dangling symbolic link that Khem mentioned, or more like not handle it, let it fall through and fail. Not that this case is going to happen very often since our 1 pipe fifo is the same and not likely going to be a link. Sau! >> I was attempting to get a point fix for the release, we can worry >> about a more robust handling of the error / upgrade case in 1.5.1 >> or 1.6. > > Righto. I am blissfully ignorant of the criteria for the release so I > am happy to defer to your judgement on that. > > p. > > > >