| Message ID | 20240705065434.3608030-2-dominique.martinet@atmark-techno.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | misc fixes | expand |
Dominique Martinet wrote on Fri, Jul 05, 2024 at 03:54:30PM +0900: > ipc_get_status_timeout() returns the size of the message on success, but > ipc_get_status() returns 0 in this case (both return a negative value on > error) > > Changing the function to use ipc_get_status() didn't update the return > value check: fix this, and also properly check for errors. > Also remove obsolete comment describing the return value. > > Reported-by: James Hilliard <james.hilliard1@gmail.com> > Fixes: da48265ad29f ("suricatta process notification: improve ipc_get_status scheduling") > Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> > --- > suricatta/server_hawkbit.c | 13 ++++++------- > 1 file changed, 6 insertions(+), 7 deletions(-) > > diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c > index 9c3e97418361..26260b6bd5e0 100644 > --- a/suricatta/server_hawkbit.c > +++ b/suricatta/server_hawkbit.c > @@ -1003,7 +1003,12 @@ static void *process_notification_thread(void *data) > bool data_avail = false; > int ret = ipc_get_status(&msg); > > - data_avail = ret > 0 && (strlen(msg.data.status.desc) != 0); > + if (ret < 0) { > + ERROR("Error getting status, stopping notification thread"); > + stop = true;; I always notice this kind of things after sending the mail... there's two ';' here It doesn't really matter, but might as well remove the second one when you apply it (not bothering to resend, please ask if you want me to)
On 05.07.24 09:02, Dominique Martinet wrote: > Dominique Martinet wrote on Fri, Jul 05, 2024 at 03:54:30PM +0900: >> ipc_get_status_timeout() returns the size of the message on success, but >> ipc_get_status() returns 0 in this case (both return a negative value on >> error) >> >> Changing the function to use ipc_get_status() didn't update the return >> value check: fix this, and also properly check for errors. >> Also remove obsolete comment describing the return value. >> >> Reported-by: James Hilliard <james.hilliard1@gmail.com> >> Fixes: da48265ad29f ("suricatta process notification: improve ipc_get_status scheduling") >> Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> >> --- >> suricatta/server_hawkbit.c | 13 ++++++------- >> 1 file changed, 6 insertions(+), 7 deletions(-) >> >> diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c >> index 9c3e97418361..26260b6bd5e0 100644 >> --- a/suricatta/server_hawkbit.c >> +++ b/suricatta/server_hawkbit.c >> @@ -1003,7 +1003,12 @@ static void *process_notification_thread(void *data) >> bool data_avail = false; >> int ret = ipc_get_status(&msg); >> >> - data_avail = ret > 0 && (strlen(msg.data.status.desc) != 0); >> + if (ret < 0) { >> + ERROR("Error getting status, stopping notification thread"); >> + stop = true;; > > I always notice this kind of things after sending the mail... there's > two ';' here > > It doesn't really matter, but might as well remove the second one when > you apply it (not bothering to resend, please ask if you want me to) > I can fix this myself. Stefano
diff --git a/suricatta/server_hawkbit.c b/suricatta/server_hawkbit.c index 9c3e97418361..26260b6bd5e0 100644 --- a/suricatta/server_hawkbit.c +++ b/suricatta/server_hawkbit.c @@ -1003,7 +1003,12 @@ static void *process_notification_thread(void *data) bool data_avail = false; int ret = ipc_get_status(&msg); - data_avail = ret > 0 && (strlen(msg.data.status.desc) != 0); + if (ret < 0) { + ERROR("Error getting status, stopping notification thread"); + stop = true;; + } else { + data_avail = (strlen(msg.data.status.desc) != 0); + } /* * Mutex used to synchronize end of the thread @@ -1017,12 +1022,6 @@ static void *process_notification_thread(void *data) if (data_avail && msg.data.status.current == PROGRESS) continue; - /* - * If there is a message - * ret > 0: data available - * ret == 0: TIMEOUT, no more messages - * ret < 0 : ERROR, exit - */ if (data_avail && numdetails < MAX_DETAILS) { for (int c = 0; c < strlen(msg.data.status.desc); c++) { switch (msg.data.status.desc[c]) {
ipc_get_status_timeout() returns the size of the message on success, but ipc_get_status() returns 0 in this case (both return a negative value on error) Changing the function to use ipc_get_status() didn't update the return value check: fix this, and also properly check for errors. Also remove obsolete comment describing the return value. Reported-by: James Hilliard <james.hilliard1@gmail.com> Fixes: da48265ad29f ("suricatta process notification: improve ipc_get_status scheduling") Signed-off-by: Dominique Martinet <dominique.martinet@atmark-techno.com> --- suricatta/server_hawkbit.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)