Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
MIME-Version: 1.0
References: <20220325071350.26668-1-wangxinpeng@uniontech.com> <c9c39f53-86cb-c4d3-5a6b-15852a765d79@molgen.mpg.de>
In-Reply-To: <c9c39f53-86cb-c4d3-5a6b-15852a765d79@molgen.mpg.de>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Fri, 25 Mar 2022 11:44:55 -0700
Message-ID: <CABBYNZLV0M4-zW4bgHXK7v0YuW33_ifB1SYw4=i9N6Y=r9zwHQ@mail.gmail.com>
Subject: Re: [PATCH v2] obexd: Fix can't receive small files sent by windows
To:     Paul Menzel <pmenzel@molgen.mpg.de>
Cc:     Xinpeng Wang <wangxinpeng@uniontech.com>,
        "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi Paul, Xinpeng,

On Fri, Mar 25, 2022 at 10:28 AM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Xinpeng,
>
>
> Am 25.03.22 um 08:13 schrieb xinpeng wang:
> > When obexd receives a file, it parses the filename and other parameters
> > when processing the packet for the first time, store filename in
> > obex_session and emit the dbus signal, The signal will be pending first.
> > then when this file is received, transfer_complete reset obex_session
> > is called.
> >
> > When using a computer with Windows 10 installed to send a file to bluez,
> > obexd will read the data through read_stream; if it is a small file, the
> > data processed for the first time is marked as final, and transfer_complete
> > reset obex_session will be called when the data is processed for the first
> > time. At this point, the dbus signal is still pending, and the dbus method
> > that requests the file path has not been processed. This will cause the upper
> > application to not be able to transfer files from the cache directory to the
> > directory specified by the user.
> >
> > To solve this problem, emit Filename's dbus signal and force it when
> > status=active.

It would be great to have the obexctl output with and without the
change, also does it change anything with respect of bigger files that
are not completed with a single packet?

> > Ways to reproduce the problem:
> > 1. Use the computer with windows 10 installed to send a small file to the
> > computer with ubuntu installed;
> > 2. file size < 10k;
> > 3. After sending, in most cases, the file is located in the ~/.cache/obexd/
> > directory. Normally, the file should be located in the ~/Download directory.
> >
> > To fix this, after applying this commit, it also needs to be modified by
> > the upper-level application. Modified to read Filename from dbus signal if
> > there is Filename in dbus signal. Otherwise, use the dbus method to request
> > Filename.
> >
> > Signed-off-by: xinpeng wang <wangxinpeng@uniontech.com>

We don't use Signed-off-by for userspace.

> > ---
> > v2: Add the steps to reproduce the bug in the submission information.
> >
> >   obexd/src/manager.c | 6 +++++-
> >   obexd/src/obex.c    | 1 +
> >   2 files changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/obexd/src/manager.c b/obexd/src/manager.c
> > index 01741fe62..2c180dc44 100644
> > --- a/obexd/src/manager.c
> > +++ b/obexd/src/manager.c
> > @@ -533,8 +533,12 @@ void manager_emit_transfer_property(struct obex_transfer *transfer,
> >   void manager_emit_transfer_started(struct obex_transfer *transfer)
> >   {
> >       transfer->status = TRANSFER_STATUS_ACTIVE;
> > +     if (!transfer->path)
> > +             return;
> >
> > -     manager_emit_transfer_property(transfer, "Status");
> > +     g_dbus_emit_property_changed_full(connection, transfer->path,
> > +                                     TRANSFER_INTERFACE, "Status",
> > +                                     G_DBUS_PROPERTY_CHANGED_FLAG_FLUSH);
> >   }
> >
> >   static void emit_transfer_completed(struct obex_transfer *transfer,
> > diff --git a/obexd/src/obex.c b/obexd/src/obex.c
> > index 3a68fd66c..c0d9e160a 100644
> > --- a/obexd/src/obex.c
> > +++ b/obexd/src/obex.c
> > @@ -720,6 +720,7 @@ int obex_put_stream_start(struct obex_session *os, const char *filename)
> >               manager_emit_transfer_property(os->service_data, "Size");
> >
> >       os->path = g_strdup(filename);
> > +     manager_emit_transfer_property(os->service_data, "Filename");
> >
> >       return 0;
> >   }
>
> Awesome. Thank you.
>
> Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>
>
>
> Kind regards,
>
> Paul


-- 
Luiz Augusto von Dentz