| Message ID | 1533631360-23581-1-git-send-email-angelo@amarulasolutions.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | core: create path for a file if path doesn't exists | expand |
Hi Angelo, On 07/08/2018 10:42, Angelo Compagnucci wrote: > Actually, when a path for a file is missing, swupdate will crash cause > it relies on existing paths. Does it crash ? Really ? It should just stop with an error code. > Changing this to create first the directory tree needed by a file then > writing it. > > Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> > --- > core/util.c | 26 +++++++++++++++++++++++++- > 1 file changed, 25 insertions(+), 1 deletion(-) > > diff --git a/core/util.c b/core/util.c > index b8910e0..092f5d5 100644 > --- a/core/util.c > +++ b/core/util.c > @@ -18,6 +18,7 @@ > #include <sys/mount.h> > #include <sys/uio.h> > #include <dirent.h> > +#include <libgen.h> > #include "swupdate.h" > #include "util.h" > #include "generated/autoconf.h" > @@ -130,13 +131,36 @@ void freeargs (char **argv) > } > } > > +static int mkpath(char *dir, mode_t mode) > +{ > + if (!dir) { > + errno = EINVAL; > + return 1; > + } > + > + if (strlen(dir) == 1 && dir[0] == '/') > + return 0; > + > + mkpath(dirname(strdupa(dir)), mode); > + > + if (mkdir(dir, mode) == -1) { > + if (errno != EEXIST) > + return 1; > + } > + return 0; > +} > + > int openfileoutput(const char *filename) > { > int fdout; > > + fdout = mkpath(dirname(strdupa(filename)), 0755); > + if (fdout < 0) > + ERROR("I cannot create path %s: %s\n", filename, strerror(errno)); > + > fdout = open(filename, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR ); > if (fdout < 0) > - ERROR("I cannot open %s %d\n", filename, errno); > + ERROR("I cannot open %s: %s\n", filename, strerror(errno)); > > return fdout; > } > Why should we do this ? This allows to generate any path on the target filesystem. The supported way to do this is to put the files you want into an archive and use the archive handler to unpack them. It creates then the paths. Best regards, Stefano Babic
On Tue, Aug 7, 2018 at 11:04 AM, Stefano Babic <sbabic@denx.de> wrote: > Hi Angelo, > > On 07/08/2018 10:42, Angelo Compagnucci wrote: >> Actually, when a path for a file is missing, swupdate will crash cause >> it relies on existing paths. > > Does it crash ? Really ? It should just stop with an error code. Yes, crashes it's not technically correct. > >> Changing this to create first the directory tree needed by a file then >> writing it. >> >> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> >> --- >> core/util.c | 26 +++++++++++++++++++++++++- >> 1 file changed, 25 insertions(+), 1 deletion(-) >> >> diff --git a/core/util.c b/core/util.c >> index b8910e0..092f5d5 100644 >> --- a/core/util.c >> +++ b/core/util.c >> @@ -18,6 +18,7 @@ >> #include <sys/mount.h> >> #include <sys/uio.h> >> #include <dirent.h> >> +#include <libgen.h> >> #include "swupdate.h" >> #include "util.h" >> #include "generated/autoconf.h" >> @@ -130,13 +131,36 @@ void freeargs (char **argv) >> } >> } >> >> +static int mkpath(char *dir, mode_t mode) >> +{ >> + if (!dir) { >> + errno = EINVAL; >> + return 1; >> + } >> + >> + if (strlen(dir) == 1 && dir[0] == '/') >> + return 0; >> + >> + mkpath(dirname(strdupa(dir)), mode); >> + >> + if (mkdir(dir, mode) == -1) { >> + if (errno != EEXIST) >> + return 1; >> + } >> + return 0; >> +} >> + >> int openfileoutput(const char *filename) >> { >> int fdout; >> >> + fdout = mkpath(dirname(strdupa(filename)), 0755); >> + if (fdout < 0) >> + ERROR("I cannot create path %s: %s\n", filename, strerror(errno)); >> + >> fdout = open(filename, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR ); >> if (fdout < 0) >> - ERROR("I cannot open %s %d\n", filename, errno); >> + ERROR("I cannot open %s: %s\n", filename, strerror(errno)); >> >> return fdout; >> } >> > > Why should we do this ? This allows to generate any path on the target > filesystem. > > The supported way to do this is to put the files you want into an > archive and use the archive handler to unpack them. It creates then the > paths. For a customer, we need to have a list of files in the cpio, some of them installed for a software collection and some for another. Using an archive is not an option here because it is needed to extract it somewhere before selecting which files to copy and requires some extra logic. Instead using a "files" section for each one of the software collections is really a good and clean way to handle it. > Best regards, > Stefano Babic > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany > Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de > =====================================================================
Hi Angelo, On 07/08/2018 11:13, Angelo Compagnucci wrote: > On Tue, Aug 7, 2018 at 11:04 AM, Stefano Babic <sbabic@denx.de> wrote: >> Hi Angelo, >> >> On 07/08/2018 10:42, Angelo Compagnucci wrote: >>> Actually, when a path for a file is missing, swupdate will crash cause >>> it relies on existing paths. >> >> Does it crash ? Really ? It should just stop with an error code. > > Yes, crashes it's not technically correct. > >> >>> Changing this to create first the directory tree needed by a file then >>> writing it. >>> >>> Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> >>> --- >>> core/util.c | 26 +++++++++++++++++++++++++- >>> 1 file changed, 25 insertions(+), 1 deletion(-) >>> >>> diff --git a/core/util.c b/core/util.c >>> index b8910e0..092f5d5 100644 >>> --- a/core/util.c >>> +++ b/core/util.c >>> @@ -18,6 +18,7 @@ >>> #include <sys/mount.h> >>> #include <sys/uio.h> >>> #include <dirent.h> >>> +#include <libgen.h> >>> #include "swupdate.h" >>> #include "util.h" >>> #include "generated/autoconf.h" >>> @@ -130,13 +131,36 @@ void freeargs (char **argv) >>> } >>> } >>> >>> +static int mkpath(char *dir, mode_t mode) >>> +{ >>> + if (!dir) { >>> + errno = EINVAL; >>> + return 1; >>> + } error is misleading - you are missing return code and errno. And the calling function is not checking errno. You should simply return -EINVAL. >>> + >>> + if (strlen(dir) == 1 && dir[0] == '/') >>> + return 0; >>> + >>> + mkpath(dirname(strdupa(dir)), mode); >>> + >>> + if (mkdir(dir, mode) == -1) { >>> + if (errno != EEXIST) >>> + return 1; >>> + } >>> + return 0; >>> +} >>> + >>> int openfileoutput(const char *filename) >>> { >>> int fdout; >>> >>> + fdout = mkpath(dirname(strdupa(filename)), 0755); >>> + if (fdout < 0) >>> + ERROR("I cannot create path %s: %s\n", filename, strerror(errno)); >>> + >>> fdout = open(filename, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR ); >>> if (fdout < 0) >>> - ERROR("I cannot open %s %d\n", filename, errno); >>> + ERROR("I cannot open %s: %s\n", filename, strerror(errno)); >>> >>> return fdout; >>> } >>> >> >> Why should we do this ? This allows to generate any path on the target >> filesystem. >> >> The supported way to do this is to put the files you want into an >> archive and use the archive handler to unpack them. It creates then the >> paths. > > For a customer, we need to have a list of files in the cpio, some of > them installed for a software collection and some for another. > Using an archive is not an option here because it is needed to extract > it somewhere before selecting which files to copy and requires some > extra logic. > Instead using a "files" section for each one of the software > collections is really a good and clean way to handle it. openfileoutput() is a central function in SWUpdate and it supposed that file can be generated. Changing this behavior can have side effects. But from your description, you need this just for the "rawfile" handler, right ? This behavior can be better controlled inside the handler - for the same reason, path is not automatically mounted if needed, but inside this handler, too. You can check inside rawfile handler if the path exists and youz can create if required. Anyway, this should be not automatically done, but ruled by a flag into sw-description. Best regards, Stefano Babic
diff --git a/core/util.c b/core/util.c index b8910e0..092f5d5 100644 --- a/core/util.c +++ b/core/util.c @@ -18,6 +18,7 @@ #include <sys/mount.h> #include <sys/uio.h> #include <dirent.h> +#include <libgen.h> #include "swupdate.h" #include "util.h" #include "generated/autoconf.h" @@ -130,13 +131,36 @@ void freeargs (char **argv) } } +static int mkpath(char *dir, mode_t mode) +{ + if (!dir) { + errno = EINVAL; + return 1; + } + + if (strlen(dir) == 1 && dir[0] == '/') + return 0; + + mkpath(dirname(strdupa(dir)), mode); + + if (mkdir(dir, mode) == -1) { + if (errno != EEXIST) + return 1; + } + return 0; +} + int openfileoutput(const char *filename) { int fdout; + fdout = mkpath(dirname(strdupa(filename)), 0755); + if (fdout < 0) + ERROR("I cannot create path %s: %s\n", filename, strerror(errno)); + fdout = open(filename, O_CREAT | O_WRONLY | O_TRUNC, S_IRUSR | S_IWUSR ); if (fdout < 0) - ERROR("I cannot open %s %d\n", filename, errno); + ERROR("I cannot open %s: %s\n", filename, strerror(errno)); return fdout; }
Actually, when a path for a file is missing, swupdate will crash cause it relies on existing paths. Changing this to create first the directory tree needed by a file then writing it. Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com> --- core/util.c | 26 +++++++++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-)