diff mbox series

[BlueZ,1/4] btio: Handle closed channel in server_cb

Message ID 20231016154900.3094-2-iulia.tanasescu@nxp.com (mailing list archive)
State New, archived
Headers show
Series Add Modify Source initial implementation | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch success CheckPatch PASS
tedd_an/GitLint success Gitlint PASS
tedd_an/BuildEll success Build ELL PASS
tedd_an/BluezMake success Bluez Make PASS
tedd_an/MakeCheck success Bluez Make Check PASS
tedd_an/MakeDistcheck success Make Distcheck PASS
tedd_an/CheckValgrind success Check Valgrind PASS
tedd_an/CheckSmatch success CheckSparse PASS
tedd_an/bluezmakeextell success Make External ELL PASS
tedd_an/IncrementalBuild success Incremental Build PASS
tedd_an/ScanBuild warning ScanBuild: btio/btio.c:1852:4: warning: Potential leak of memory pointed to by 'addr' ERROR_FAILED(err, "bind", errno); ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ btio/btio.c:41:3: note: expanded from macro 'ERROR_FAILED' g_set_error(gerr, BT_IO_ERROR, err, \ ^~~~~~~~~~~ 1 warning generated. src/shared/bass.c:1294:3: warning: Potential leak of memory pointed to by 'subgroup_data' return; ^~~~~~ 1 warning generated.

Commit Message

Iulia Tanasescu Oct. 16, 2023, 3:48 p.m. UTC
This handles G_IO_ERR and G_IO_HUP conditions in server_cb

---
 btio/btio.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Luiz Augusto von Dentz Oct. 16, 2023, 5 p.m. UTC | #1
Hi Iulia,

On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com> wrote:
>
> This handles G_IO_ERR and G_IO_HUP conditions in server_cb
>
> ---
>  btio/btio.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/btio/btio.c b/btio/btio.c
> index d45b8240d..c63a6d1df 100644
> --- a/btio/btio.c
> +++ b/btio/btio.c
> @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io, GIOCondition cond,
>         GIOChannel *cli_io;
>
>         /* If the user closed the server */
> -       if ((cond & G_IO_NVAL) || check_nval(io))
> +       if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> +                       check_nval(io))

I believe this was done on purpose to only check for NVAL, the other
conditions shall probably be notified back via callback so it can
process the error reported.

>                 return FALSE;
>
>         srv_sock = g_io_channel_unix_get_fd(io);
> --
> 2.39.2
>
bluez.test.bot@gmail.com Oct. 16, 2023, 5:53 p.m. UTC | #2
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=793607

---Test result---

Test Summary:
CheckPatch                    PASS      2.06 seconds
GitLint                       PASS      1.10 seconds
BuildEll                      PASS      27.73 seconds
BluezMake                     PASS      798.70 seconds
MakeCheck                     PASS      12.22 seconds
MakeDistcheck                 PASS      175.13 seconds
CheckValgrind                 PASS      267.39 seconds
CheckSmatch                   PASS      360.77 seconds
bluezmakeextell               PASS      115.59 seconds
IncrementalBuild              PASS      2814.91 seconds
ScanBuild                     WARNING   1102.61 seconds

Details
##############################
Test: ScanBuild - WARNING
Desc: Run Scan Build
Output:
btio/btio.c:1852:4: warning: Potential leak of memory pointed to by 'addr'
                        ERROR_FAILED(err, "bind", errno);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
btio/btio.c:41:3: note: expanded from macro 'ERROR_FAILED'
                g_set_error(gerr, BT_IO_ERROR, err, \
                ^~~~~~~~~~~
1 warning generated.
src/shared/bass.c:1294:3: warning: Potential leak of memory pointed to by 'subgroup_data'
                return;
                ^~~~~~
1 warning generated.



---
Regards,
Linux Bluetooth
Iulia Tanasescu Oct. 25, 2023, 2:55 p.m. UTC | #3
Hi Luiz,

> -----Original Message-----
> From: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
> Sent: Monday, October 16, 2023 8:01 PM
> To: Iulia Tanasescu <iulia.tanasescu@nxp.com>
> Cc: linux-bluetooth@vger.kernel.org; Claudia Cristina Draghicescu
> <claudia.rosu@nxp.com>; Mihai-Octavian Urzica <mihai-
> octavian.urzica@nxp.com>; Silviu Florian Barbulescu
> <silviu.barbulescu@nxp.com>; Vlad Pruteanu <vlad.pruteanu@nxp.com>;
> Andrei Istodorescu <andrei.istodorescu@nxp.com>
> Subject: Re: [PATCH BlueZ 1/4] btio: Handle closed channel in server_cb
> 
> Hi Iulia,
> 
> On Mon, Oct 16, 2023 at 8:49 AM Iulia Tanasescu <iulia.tanasescu@nxp.com>
> wrote:
> >
> > This handles G_IO_ERR and G_IO_HUP conditions in server_cb
> >
> > ---
> >  btio/btio.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/btio/btio.c b/btio/btio.c index d45b8240d..c63a6d1df
> > 100644
> > --- a/btio/btio.c
> > +++ b/btio/btio.c
> > @@ -247,7 +247,8 @@ static gboolean server_cb(GIOChannel *io,
> GIOCondition cond,
> >         GIOChannel *cli_io;
> >
> >         /* If the user closed the server */
> > -       if ((cond & G_IO_NVAL) || check_nval(io))
> > +       if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
> > +                       check_nval(io))
> 
> I believe this was done on purpose to only check for NVAL, the other
> conditions shall probably be notified back via callback so it can process the
> error reported.
> 

It seems that server_cb is supposed to accept an incoming connection on
server io and pass the child io through the confirm or connect callbacks.
But if the condition is G_IO_HUP for example, we shouldn't get to the point
of calling accept.

> >                 return FALSE;
> >
> >         srv_sock = g_io_channel_unix_get_fd(io);
> > --
> > 2.39.2
> >
> 
> 
> --
> Luiz Augusto von Dentz

Regards,
Iulia
diff mbox series

Patch

diff --git a/btio/btio.c b/btio/btio.c
index d45b8240d..c63a6d1df 100644
--- a/btio/btio.c
+++ b/btio/btio.c
@@ -247,7 +247,8 @@  static gboolean server_cb(GIOChannel *io, GIOCondition cond,
 	GIOChannel *cli_io;
 
 	/* If the user closed the server */
-	if ((cond & G_IO_NVAL) || check_nval(io))
+	if ((cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) ||
+			check_nval(io))
 		return FALSE;
 
 	srv_sock = g_io_channel_unix_get_fd(io);