Hi, On Thu, Jun 27, 2024 at 12:24 PM Pauli Virtanen <pav@xxxxxx> wrote: > > Hi, > > ke, 2024-06-26 kello 11:40 +0530, Amisha Jain kirjoitti: > > Hi, > > > > Correct! The problem here is manager_emit_transfer_property() is > > expecting the structure of type 'obex_transfer' and we are passing the > > structure of session type which will be type mismatch and inappropriate > > values will be populated in further calls in code. Hence property "Size" > > will never emit on console(obexctl) as it is not set properly and might > > cause crash/disconnection. > > > > > diff --git a/obexd/src/obex.c b/obexd/src/obex.c > > > index a4bae857f..ed219d3e7 100644 > > > --- a/obexd/src/obex.c > > > +++ b/obexd/src/obex.c > > > @@ -779,6 +779,9 @@ int obex_put_stream_start(struct obex_session > > *os, const char *filename) > > > return err; > > > } > > > > > > + if (os->size != OBJECT_SIZE_DELETE && os->size != > > OBJECT_SIZE_UNKNOWN) > > > + manager_emit_transfer_property(os->service_data, "Size"); > > > + > > > os->path = g_strdup(filename); > > > > > > > > > One way to resolve this issue is to add the additional field in > > 'mas_session' so it can cast to struct 'obex_transfer'. We are adding > > new field 'char *path' as only transfer->path will be invoked and passed > > further. > > Yes, thank you for clarification. > > Although I'm not BlueZ maintainer, I think relying on type punning > mas_session and obex_transfer by adding fields so that their memory > representation is partly the same is not very good practice. Other > plugins like ftp.c seem also have similar issue. Yeah, it clearly was not intended to be that way. > One could consider some ways to avoid doing this, e.g. move the > > if (os->size != OBJECT_SIZE_DELETE && os->size != OBJECT_SIZE_UNKNOWN) > manager_emit_transfer_property(os->service_data, "Size"); > > after each call to obex_put_stream_start in plugins/*.c > client/mns.c. For plugins where there's no transfer, it maybe can be > omitted. I think this was intended to be called directly by the plugins otherwise we need to always create transfer objects for each object being transferred and that be set as os->service_data which obviously isn't the case here. > > > > void manager_emit_transfer_property(struct obex_transfer *transfer, > > char *name) > > { > > if (!transfer->path) > > return; > > > > g_dbus_emit_property_changed(connection, transfer->path, > > TRANSFER_INTERFACE, name); > > } > > > > >> Signed-off-by: Amisha Jain <quic_amisjain@xxxxxxxxxxx> > > >> --- > > >> obexd/plugins/mas.c | 4 +++- > > >> 1 file changed, 3 insertions(+), 1 deletion(-) > > >> > > >> diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c > > >> index 10b972d65..71bf12ad3 100644 > > >> --- a/obexd/plugins/mas.c > > >> +++ b/obexd/plugins/mas.c > > >> @@ -51,6 +51,8 @@ > > >> #define ML_BODY_END "</MAP-msg-listing>" > > >> > > >> struct mas_session { > > >> + uint8_t notification_status; > > >> + char *path; > > >> struct mas_request *request; > > >> void *backend_data; > > >> gboolean finished; > > >> @@ -59,7 +61,6 @@ struct mas_session { > > >> GObexApparam *inparams; > > >> GObexApparam *outparams; > > >> gboolean ap_sent; > > >> - uint8_t notification_status; > > >> }; > > >> > > >> static const uint8_t MAS_TARGET[TARGET_SIZE] = { > > >> @@ -125,6 +126,7 @@ static void *mas_connect(struct obex_session > > *os, int *err) > > >> goto failed; > > >> > > >> manager_register_session(os); > > >> + mas->path = NULL; > > > > > > > There is no transfer already registered for mas during connection, so > > setting the path to NULL. > > > > On 6/25/2024 9:09 PM, Pauli Virtanen wrote: > > > Hi, > > > > > > ti, 2024-06-25 kello 12:05 +0530, Amisha Jain kirjoitti: > > > > Update the 'mas_session' structure such that > > > > manager_emit_transfer_property(os->service_data, "Size") > > > > will get the proper structure in arguments as > > > > expected like structure 'obex_transfer' and transfer->path > > > > won't be populated with inappropriate value. > > > > > > > > As there is no new transfer registered during mas connect, > > > > hence setting the path to NULL to avoid invoking the > > > > g_dbus_emit_property_changed() property. > > > > > > > > Signed-off-by: Amisha Jain <quic_amisjain@xxxxxxxxxxx> > > > > --- > > > > obexd/plugins/mas.c | 4 +++- > > > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/obexd/plugins/mas.c b/obexd/plugins/mas.c > > > > index 10b972d65..71bf12ad3 100644 > > > > --- a/obexd/plugins/mas.c > > > > +++ b/obexd/plugins/mas.c > > > > @@ -51,6 +51,8 @@ > > > > #define ML_BODY_END "</MAP-msg-listing>" > > > > > > > > struct mas_session { > > > > + uint8_t notification_status; > > > > + char *path; > > > > struct mas_request *request; > > > > void *backend_data; > > > > gboolean finished; > > > > @@ -59,7 +61,6 @@ struct mas_session { > > > > GObexApparam *inparams; > > > > GObexApparam *outparams; > > > > gboolean ap_sent; > > > > - uint8_t notification_status; > > > > }; > > > > > > > > static const uint8_t MAS_TARGET[TARGET_SIZE] = { > > > > @@ -125,6 +126,7 @@ static void *mas_connect(struct obex_session *os, int *err) > > > > goto failed; > > > > > > > > manager_register_session(os); > > > > + mas->path = NULL; > > > > > > Maybe the problem here is that the change in commit bb160515185e > > > ("obexd: Emit Size property of transfer after open()") is not right? > > > > > > diff --git a/obexd/src/obex.c b/obexd/src/obex.c > > > index a4bae857f..ed219d3e7 100644 > > > --- a/obexd/src/obex.c > > > +++ b/obexd/src/obex.c > > > @@ -779,6 +779,9 @@ int obex_put_stream_start(struct obex_session *os, const char *filename) > > > return err; > > > } > > > > > > + if (os->size != OBJECT_SIZE_DELETE && os->size != OBJECT_SIZE_UNKNOWN) > > > + manager_emit_transfer_property(os->service_data, "Size"); > > > + > > > os->path = g_strdup(filename); > > > > > > This casts os->service_data to obex_transfer which IIUC does not work > > > for most the plugins, as it's the session struct. > > > > > > Maybe plugins can emit the transfer property change in their open() > > > callback, for the plugins where it makes sense? > > > > > > > > > > > return mas; > > > > > > > > > -- > Pauli Virtanen > -- Luiz Augusto von Dentz