| Message ID | YfAhEo77OfI7NiRo@decadent.org.uk |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [1/2] installer: Rename extract_scripts() and update comments | expand |
Hi Ben, On 25.01.22 17:10, Ben Hutchings wrote: > Scripts are now extracted by the cpio streaming loop, and this > function only moveds them. Rename it to move_scripts() and update > the comments to show what's actually going on. > I do not know if this brings some added value, and if "move" is the correct word about what function is doing. In fact, extract_scripts() (or move_scripts() after your patch) is copying (and not moving) the scripts (if any), see call to copyfile(). What the respon for this change of name ? Best regards, Stefano Babic > Signed-off-by: Ben Hutchings <ben.hutchings_ext@softathome.com> > --- > core/installer.c | 11 +++++------ > 1 file changed, 5 insertions(+), 6 deletions(-) > > diff --git a/core/installer.c b/core/installer.c > index eb07f41..089f638 100644 > --- a/core/installer.c > +++ b/core/installer.c > @@ -89,10 +89,9 @@ swupdate_file_t check_if_required(struct imglist *list, struct filehdr *pfdh, > > > /* > - * Extract all scripts from a list from the image > - * and save them on the filesystem to be executed later > + * Move all scripts from a list to the temporary scripts directory > */ > -static int extract_scripts(struct imglist *head) > +static int move_scripts(struct imglist *head) > { > struct img_type *script; > int fdout; > @@ -258,10 +257,10 @@ int install_images(struct swupdate_cfg *sw) > bool dry_run = sw->parms.dry_run; > bool dropimg; > > - /* Extract all scripts, preinstall scripts must be run now */ > + /* Move all scripts, preinstall scripts must be run now */ > const char* tmpdir_scripts = get_tmpdirscripts(); > - ret = extract_scripts(&sw->scripts); > - ret |= extract_scripts(&sw->bootscripts); > + ret = move_scripts(&sw->scripts); > + ret |= move_scripts(&sw->bootscripts); > if (ret) { > ERROR("extracting script to %s failed", tmpdir_scripts); > return ret;
Stefano Babic wrote: > Hi Ben, > > On 25.01.22 17:10, Ben Hutchings wrote: > > Scripts are now extracted by the cpio streaming loop, and this > > function only moveds them. Rename it to move_scripts() and update > > the comments to show what's actually going on. > > > > I do not know if this brings some added value, and if "move" is the > correct word about what function is doing. > > In fact, extract_scripts() (or move_scripts() after your patch) is > copying (and not moving) the scripts (if any), see call to copyfile(). > What the respon for this change of name ? As I said, it's no longer extracting files so the name is misleading. But as you point out, it's copying files so "move_scripts" is also misleading. So I could change it to copy_scripts instead. Ben.
On 26.01.22 16:13, Ben HUTCHINGS (EXT) wrote: > Stefano Babic wrote: >> Hi Ben, >> >> On 25.01.22 17:10, Ben Hutchings wrote: >>> Scripts are now extracted by the cpio streaming loop, and this >>> function only moveds them. Rename it to move_scripts() and update >>> the comments to show what's actually going on. >>> >> >> I do not know if this brings some added value, and if "move" is the >> correct word about what function is doing. >> >> In fact, extract_scripts() (or move_scripts() after your patch) is >> copying (and not moving) the scripts (if any), see call to copyfile(). >> What the respon for this change of name ? > > As I said, it's no longer extracting files so the name is misleading. Ok > But as > you point out, it's copying files so "move_scripts" is also misleading. Right. > So I > could change it to copy_scripts instead. Fine. Best regards, Stefano > > Ben. >
diff --git a/core/installer.c b/core/installer.c index eb07f41..089f638 100644 --- a/core/installer.c +++ b/core/installer.c @@ -89,10 +89,9 @@ swupdate_file_t check_if_required(struct imglist *list, struct filehdr *pfdh, /* - * Extract all scripts from a list from the image - * and save them on the filesystem to be executed later + * Move all scripts from a list to the temporary scripts directory */ -static int extract_scripts(struct imglist *head) +static int move_scripts(struct imglist *head) { struct img_type *script; int fdout; @@ -258,10 +257,10 @@ int install_images(struct swupdate_cfg *sw) bool dry_run = sw->parms.dry_run; bool dropimg; - /* Extract all scripts, preinstall scripts must be run now */ + /* Move all scripts, preinstall scripts must be run now */ const char* tmpdir_scripts = get_tmpdirscripts(); - ret = extract_scripts(&sw->scripts); - ret |= extract_scripts(&sw->bootscripts); + ret = move_scripts(&sw->scripts); + ret |= move_scripts(&sw->bootscripts); if (ret) { ERROR("extracting script to %s failed", tmpdir_scripts); return ret;
Scripts are now extracted by the cpio streaming loop, and this function only moveds them. Rename it to move_scripts() and update the comments to show what's actually going on. Signed-off-by: Ben Hutchings <ben.hutchings_ext@softathome.com> --- core/installer.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-)