| Message ID | 20220727130030.1607431-1-ayoub.zaki@embexus.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [V2] replace system("reboot") calls by native glibc call reboot() | expand |
On 27.07.22 15:00, Ayoub Zaki wrote: > Signed-off-by: Ayoub Zaki <ayoub.zaki@embexus.com> > --- > tools/swupdate-ipc.c | 6 ++++-- > tools/swupdate-progress.c | 6 ++++-- > 2 files changed, 8 insertions(+), 4 deletions(-) > > diff --git a/tools/swupdate-ipc.c b/tools/swupdate-ipc.c > index ce8ec10..903c7e9 100644 > --- a/tools/swupdate-ipc.c > +++ b/tools/swupdate-ipc.c > @@ -23,6 +23,8 @@ > #include <sys/stat.h> > #include <sys/un.h> > #include <sys/select.h> > +#include <sys/reboot.h> > +#include <linux/reboot.h> > #include <arpa/inet.h> > #include <netinet/in.h> > #include <ifaddrs.h> > @@ -570,9 +572,9 @@ static int sysrestart(cmd_t __attribute__((__unused__)) *cmd, int argc, char *a > case SUCCESS: > fprintf(stdout, "Ready to reboot !\n"); > restart_system(ndevs); > - sleep(5); > + sync(); There are some reasons to delay a restart. One of them is that the reboot itself is demanded by one of these tools, but the application (running with lower rights) was also informed (via progress interface or whatever) and have time to gracefully shutdown. That means it is not just a delay to allow filesystem sync. If this delay is disturbing for you, I suggest you introduce (with a follow-up patch) a parameter like "-t <timeout>" to delay the reboot, like a "shutdown -t..". > > - if (system("reboot") < 0) { /* It should never happen */ > + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen */ > fprintf(stdout, "Please reset the board.\n"); > } > break; > diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c > index 8910b5a..e45e732 100644 > --- a/tools/swupdate-progress.c > +++ b/tools/swupdate-progress.c > @@ -18,6 +18,8 @@ > #include <sys/stat.h> > #include <sys/un.h> > #include <sys/select.h> > +#include <sys/reboot.h> > +#include <linux/reboot.h> > #include <arpa/inet.h> > #include <netinet/in.h> > #include <pthread.h> > @@ -379,8 +381,8 @@ int main(int argc, char **argv) > psplash_progress(psplash_pipe_path, &msg); > psplash_ok = 0; > if ((msg.status == SUCCESS) && (msg.cur_step > 0) && opt_r) { > - sleep(5); > - if (system("reboot") < 0) { /* It should never happen */ > + sync(); > + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen */ > fprintf(stdout, "Please reset the board.\n"); > } > } Best regards, Stefano
Hi Stefano, thanks for your insight, I don't mind having a sleep(5), follows a V3 version. Best regards On Thursday, July 28, 2022 at 8:50:48 AM UTC+2 Stefano Babic wrote: > On 27.07.22 15:00, Ayoub Zaki wrote: > > Signed-off-by: Ayoub Zaki <ayoub...@embexus.com> > > --- > > tools/swupdate-ipc.c | 6 ++++-- > > tools/swupdate-progress.c | 6 ++++-- > > 2 files changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/tools/swupdate-ipc.c b/tools/swupdate-ipc.c > > index ce8ec10..903c7e9 100644 > > --- a/tools/swupdate-ipc.c > > +++ b/tools/swupdate-ipc.c > > @@ -23,6 +23,8 @@ > > #include <sys/stat.h> > > #include <sys/un.h> > > #include <sys/select.h> > > +#include <sys/reboot.h> > > +#include <linux/reboot.h> > > #include <arpa/inet.h> > > #include <netinet/in.h> > > #include <ifaddrs.h> > > @@ -570,9 +572,9 @@ static int sysrestart(cmd_t > __attribute__((__unused__)) *cmd, int argc, char *a > > case SUCCESS: > > fprintf(stdout, "Ready to reboot !\n"); > > restart_system(ndevs); > > - sleep(5); > > + sync(); > > There are some reasons to delay a restart. One of them is that the > reboot itself is demanded by one of these tools, but the application > (running with lower rights) was also informed (via progress interface or > whatever) and have time to gracefully shutdown. That means it is not > just a delay to allow filesystem sync. > > If this delay is disturbing for you, I suggest you introduce (with a > follow-up patch) a parameter like "-t <timeout>" to delay the reboot, > like a "shutdown -t..". > > > > > - if (system("reboot") < 0) { /* It should never happen */ > > + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen > */ > > fprintf(stdout, "Please reset the board.\n"); > > } > > break; > > diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c > > index 8910b5a..e45e732 100644 > > --- a/tools/swupdate-progress.c > > +++ b/tools/swupdate-progress.c > > @@ -18,6 +18,8 @@ > > #include <sys/stat.h> > > #include <sys/un.h> > > #include <sys/select.h> > > +#include <sys/reboot.h> > > +#include <linux/reboot.h> > > #include <arpa/inet.h> > > #include <netinet/in.h> > > #include <pthread.h> > > @@ -379,8 +381,8 @@ int main(int argc, char **argv) > > psplash_progress(psplash_pipe_path, &msg); > > psplash_ok = 0; > > if ((msg.status == SUCCESS) && (msg.cur_step > 0) && opt_r) { > > - sleep(5); > > - if (system("reboot") < 0) { /* It should never happen */ > > + sync(); > > + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen > */ > > fprintf(stdout, "Please reset the board.\n"); > > } > > } > > > Best regards, > Stefano > > -- > ===================================================================== > DENX Software Engineering GmbH, Managing Director: Wolfgang Denk > HRB 165235 Munich, Office: Kirchenstr.5, 82194 Groebenzell, Germany > Phone: +49-8142-66989-53 <+49%208142%206698953> Fax: +49-8142-66989-80 > <+49%208142%206698980> Email: sba...@denx.de > ===================================================================== >
diff --git a/tools/swupdate-ipc.c b/tools/swupdate-ipc.c index ce8ec10..903c7e9 100644 --- a/tools/swupdate-ipc.c +++ b/tools/swupdate-ipc.c @@ -23,6 +23,8 @@ #include <sys/stat.h> #include <sys/un.h> #include <sys/select.h> +#include <sys/reboot.h> +#include <linux/reboot.h> #include <arpa/inet.h> #include <netinet/in.h> #include <ifaddrs.h> @@ -570,9 +572,9 @@ static int sysrestart(cmd_t __attribute__((__unused__)) *cmd, int argc, char *a case SUCCESS: fprintf(stdout, "Ready to reboot !\n"); restart_system(ndevs); - sleep(5); + sync(); - if (system("reboot") < 0) { /* It should never happen */ + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen */ fprintf(stdout, "Please reset the board.\n"); } break; diff --git a/tools/swupdate-progress.c b/tools/swupdate-progress.c index 8910b5a..e45e732 100644 --- a/tools/swupdate-progress.c +++ b/tools/swupdate-progress.c @@ -18,6 +18,8 @@ #include <sys/stat.h> #include <sys/un.h> #include <sys/select.h> +#include <sys/reboot.h> +#include <linux/reboot.h> #include <arpa/inet.h> #include <netinet/in.h> #include <pthread.h> @@ -379,8 +381,8 @@ int main(int argc, char **argv) psplash_progress(psplash_pipe_path, &msg); psplash_ok = 0; if ((msg.status == SUCCESS) && (msg.cur_step > 0) && opt_r) { - sleep(5); - if (system("reboot") < 0) { /* It should never happen */ + sync(); + if (reboot(LINUX_REBOOT_CMD_RESTART) < 0) { /* It should never happen */ fprintf(stdout, "Please reset the board.\n"); } }
Signed-off-by: Ayoub Zaki <ayoub.zaki@embexus.com> --- tools/swupdate-ipc.c | 6 ++++-- tools/swupdate-progress.c | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-)