| Message ID | 20181214081258.1763273-1-pn@denx.de |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | ipc: add custom argument for client callbacks | expand |
Hi Parthiban, On 14/12/18 09:12, Parthiban Nallathambi wrote: > current implementation of swupdate_async_start doesn't take > custom argument for callbacks (read, finish). This leaves > consumer of this API to maintain global memory always. > > Added extra argument to swupdate_async_start and writedata, > terminated callsback with this argument. This makes consumer > of this API to use custom memory and preserve arguments for > later usage. Yes, it is a pity that I have not thought in advance to add an argument for customer data. Anyway, this breaks API (I guess this cannot be avoided) and a user cannot know if API was changed. A custom application cannot know even at buold time if API was changed. We need at least to signal this. I guess we need to introduce a API version number in network_ipc.h to signal this, something like #define SWUPDATE_API <nummer> Current applications can work-around the changes with something like: #if !(defined(SWUPDATE_API) rc = swupdate_async_start(readimage, printstatus, end, false); #else rc = swupdate_async_start(readimage, printstatus, end, NULL, false); #endif If we have not, it is a nightmare for users of the library to understand which version is running. > > Signed-off-by: Parthiban Nallathambi <pn@denx.de> > --- > include/network_ipc.h | 6 +++--- > ipc/network_ipc.c | 8 +++++--- > tools/client.c | 6 +++--- > 3 files changed, 11 insertions(+), 9 deletions(-) > > diff --git a/include/network_ipc.h b/include/network_ipc.h > index ea997b8..d9540ed 100644 > --- a/include/network_ipc.h > +++ b/include/network_ipc.h > @@ -69,12 +69,12 @@ int ipc_get_status(ipc_message *msg); > int ipc_postupdate(ipc_message *msg); > int ipc_send_cmd(ipc_message *msg); > > -typedef int (*writedata)(char **buf, int *size); > +typedef int (*writedata)(char **buf, int *size, void *arg); > typedef int (*getstatus)(ipc_message *msg); > -typedef int (*terminated)(RECOVERY_STATUS status); > +typedef int (*terminated)(RECOVERY_STATUS status, void *arg); > int ipc_wait_for_complete(getstatus callback); > int swupdate_image_write(char *buf, int size); > int swupdate_async_start(writedata wr_func, getstatus status_func, > - terminated end_func, bool dryrun); > + terminated end_func, void *arg, bool dryrun); > > #endif > diff --git a/ipc/network_ipc.c b/ipc/network_ipc.c > index 4cfe2f3..d9a481f 100644 > --- a/ipc/network_ipc.c > +++ b/ipc/network_ipc.c > @@ -42,6 +42,7 @@ struct async_lib { > writedata wr; > getstatus get; > terminated end; > + void *arg; > }; > > static int handle = 0; > @@ -291,7 +292,7 @@ static void *swupdate_async_thread(void *data) > if (!rq->wr) > break; > > - rq->wr(&pbuf, &size); > + rq->wr(&pbuf, &size, rq->arg); > if (size) > swupdate_image_write(pbuf, size); > } while(size > 0); > @@ -316,7 +317,7 @@ static void *swupdate_async_thread(void *data) > } > > if (rq->end) > - rq->end((RECOVERY_STATUS)swupdate_result); > + rq->end((RECOVERY_STATUS)swupdate_result, rq->arg); > > pthread_exit(NULL); > } > @@ -347,7 +348,7 @@ static pthread_t start_ipc_thread(void *(* start_routine) (void *), void *arg) > * Only one running request is accepted > */ > int swupdate_async_start(writedata wr_func, getstatus status_func, > - terminated end_func, bool dryrun) > + terminated end_func, void *arg, bool dryrun) > { > struct async_lib *rq; > int connfd; > @@ -360,6 +361,7 @@ int swupdate_async_start(writedata wr_func, getstatus status_func, > rq->wr = wr_func; > rq->get = status_func; > rq->end = end_func; > + rq->arg = arg; > > connfd = ipc_inst_start_ext(SOURCE_UNKNOWN, 0, NULL, dryrun); > > diff --git a/tools/client.c b/tools/client.c > index 1a59d97..7a0db59 100644 > --- a/tools/client.c > +++ b/tools/client.c > @@ -49,7 +49,7 @@ pthread_mutex_t mymutex; > * It is called by a thread generated by the library and > * can block. > */ > -static int readimage(char **p, int *size) { > +static int readimage(char **p, int *size, void *arg) { > int ret; > > ret = read(fd, buf, sizeof(buf)); > @@ -79,7 +79,7 @@ static int printstatus(ipc_message *msg) > * this is called at the end reporting the status > * of the upgrade > */ > -static int end(RECOVERY_STATUS status) > +static int end(RECOVERY_STATUS status, void *arg) > { > printf("Swupdate %s\n", > status == FAILURE ? "*failed* !" : > @@ -103,7 +103,7 @@ static int send_file(const char* filename) { > /* synchronize with a mutex */ > pthread_mutex_lock(&mymutex); > rc = swupdate_async_start(readimage, printstatus, > - end, false); > + end, NULL, false); > if (rc) > printf("swupdate_async_start returns %d\n", rc); > > Best regards, Stefano
diff --git a/include/network_ipc.h b/include/network_ipc.h index ea997b8..d9540ed 100644 --- a/include/network_ipc.h +++ b/include/network_ipc.h @@ -69,12 +69,12 @@ int ipc_get_status(ipc_message *msg); int ipc_postupdate(ipc_message *msg); int ipc_send_cmd(ipc_message *msg); -typedef int (*writedata)(char **buf, int *size); +typedef int (*writedata)(char **buf, int *size, void *arg); typedef int (*getstatus)(ipc_message *msg); -typedef int (*terminated)(RECOVERY_STATUS status); +typedef int (*terminated)(RECOVERY_STATUS status, void *arg); int ipc_wait_for_complete(getstatus callback); int swupdate_image_write(char *buf, int size); int swupdate_async_start(writedata wr_func, getstatus status_func, - terminated end_func, bool dryrun); + terminated end_func, void *arg, bool dryrun); #endif diff --git a/ipc/network_ipc.c b/ipc/network_ipc.c index 4cfe2f3..d9a481f 100644 --- a/ipc/network_ipc.c +++ b/ipc/network_ipc.c @@ -42,6 +42,7 @@ struct async_lib { writedata wr; getstatus get; terminated end; + void *arg; }; static int handle = 0; @@ -291,7 +292,7 @@ static void *swupdate_async_thread(void *data) if (!rq->wr) break; - rq->wr(&pbuf, &size); + rq->wr(&pbuf, &size, rq->arg); if (size) swupdate_image_write(pbuf, size); } while(size > 0); @@ -316,7 +317,7 @@ static void *swupdate_async_thread(void *data) } if (rq->end) - rq->end((RECOVERY_STATUS)swupdate_result); + rq->end((RECOVERY_STATUS)swupdate_result, rq->arg); pthread_exit(NULL); } @@ -347,7 +348,7 @@ static pthread_t start_ipc_thread(void *(* start_routine) (void *), void *arg) * Only one running request is accepted */ int swupdate_async_start(writedata wr_func, getstatus status_func, - terminated end_func, bool dryrun) + terminated end_func, void *arg, bool dryrun) { struct async_lib *rq; int connfd; @@ -360,6 +361,7 @@ int swupdate_async_start(writedata wr_func, getstatus status_func, rq->wr = wr_func; rq->get = status_func; rq->end = end_func; + rq->arg = arg; connfd = ipc_inst_start_ext(SOURCE_UNKNOWN, 0, NULL, dryrun); diff --git a/tools/client.c b/tools/client.c index 1a59d97..7a0db59 100644 --- a/tools/client.c +++ b/tools/client.c @@ -49,7 +49,7 @@ pthread_mutex_t mymutex; * It is called by a thread generated by the library and * can block. */ -static int readimage(char **p, int *size) { +static int readimage(char **p, int *size, void *arg) { int ret; ret = read(fd, buf, sizeof(buf)); @@ -79,7 +79,7 @@ static int printstatus(ipc_message *msg) * this is called at the end reporting the status * of the upgrade */ -static int end(RECOVERY_STATUS status) +static int end(RECOVERY_STATUS status, void *arg) { printf("Swupdate %s\n", status == FAILURE ? "*failed* !" : @@ -103,7 +103,7 @@ static int send_file(const char* filename) { /* synchronize with a mutex */ pthread_mutex_lock(&mymutex); rc = swupdate_async_start(readimage, printstatus, - end, false); + end, NULL, false); if (rc) printf("swupdate_async_start returns %d\n", rc);
current implementation of swupdate_async_start doesn't take custom argument for callbacks (read, finish). This leaves consumer of this API to maintain global memory always. Added extra argument to swupdate_async_start and writedata, terminated callsback with this argument. This makes consumer of this API to use custom memory and preserve arguments for later usage. Signed-off-by: Parthiban Nallathambi <pn@denx.de> --- include/network_ipc.h | 6 +++--- ipc/network_ipc.c | 8 +++++--- tools/client.c | 6 +++--- 3 files changed, 11 insertions(+), 9 deletions(-)