makedevs: Do not return error if the fifo exisits
PATCH v2 1/1 makedevs: Do not return error if the fifo exisits
This ensures that makedevs will not cause image creation failures when it encounters a pipe (fifo) that exists from a previous image. This handles mode changes and it will correctly fail for dangling symlinks.
[YOCTO #5288]
makedev
makedev
--- .../makedevs/makedevs-1.0.0/makedevs.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-)
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +14 -2
@@ -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);
Signature · 1 line
1.7.10.4
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c static void add_new_file(char *name, char *path, unsigned long uid, · line 281
Quoted 5 lines · Saul Wold
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +5
-0+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.
Quoted 11 lines · Phil Blundell
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c static void add_new_file(char *name, char *path, unsigned long uid, · line 281
Quoted 5 lines · Saul Wold
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +5
-0+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 thememset()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.
Quoted 4 lines
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.
Quoted 3 lines
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!
Quoted 5 lines
p.
Quoted 12 lines · Saul Wold
Quoted 11 lines · Phil Blundell
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c static void add_new_file(char *name, char *path, unsigned long uid, · line 281
Quoted 5 lines · Saul Wold
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +5
-0+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 thememset()may be redundant.
Quoted 4 lines
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?
Quoted 3 lines
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.
Quoted 13 lines · Phil Blundell
Quoted 12 lines · Saul Wold
Quoted 11 lines · Phil Blundell
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c static void add_new_file(char *name, char *path, unsigned long uid, · line 281
Quoted 5 lines · Saul Wold
meta/recipes-devtools/makedevs/makedevs-1.0.0/makedevs.c +5
-0+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 thememset()may be redundant.
Quoted 8 lines
Quoted 4 lines
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 thanlstat()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!
Quoted 12 lines
Quoted 3 lines
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.