diff mbox series

[BlueZ] lib: Replace malloc/memset(..0..) with malloc0

Message ID 20200928192002.22733-1-machiry@cs.ucsb.edu (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [BlueZ] lib: Replace malloc/memset(..0..) with malloc0 | expand

Commit Message

Aravind Machiry Sept. 28, 2020, 7:20 p.m. UTC
This patch replaces various instances of malloc and subsequent
memset(..,0,..) with malloc0 (i.e., calloc) for efficiency.
---
 lib/bluetooth.c |  5 +++++
 lib/bluetooth.h |  3 +++
 lib/sdp.c       | 36 ++++++++++++------------------------
 3 files changed, 20 insertions(+), 24 deletions(-)

Comments

bluez.test.bot@gmail.com Sept. 28, 2020, 7:30 p.m. UTC | #1
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=356715

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
Aravind Machiry Oct. 3, 2020, 6:11 p.m. UTC | #2
Gentle reminder!

On Mon, Sep 28, 2020 at 12:30 PM <bluez.test.bot@gmail.com> wrote:
>
> 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=356715
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - PASS
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS
>
>
>
> ---
> Regards,
> Linux Bluetooth
>
Luiz Augusto von Dentz Oct. 3, 2020, 9:49 p.m. UTC | #3
Hi Aravind,

On Sat, Oct 3, 2020 at 11:14 AM Aravind Machiry <machiry@cs.ucsb.edu> wrote:
>
> Gentle reminder!
>
> On Mon, Sep 28, 2020 at 12:30 PM <bluez.test.bot@gmail.com> wrote:
> >
> > 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=356715
> >
> > ---Test result---
> >
> > ##############################
> > Test: CheckPatch - PASS
> >
> > ##############################
> > Test: CheckGitLint - PASS
> >
> > ##############################
> > Test: CheckBuild - PASS
> >
> > ##############################
> > Test: MakeCheck - PASS
> >
> >
> >
> > ---
> > Regards,
> > Linux Bluetooth

There seems to be a mixture of malloc0 or bt_malloc0 when I guess the
later should be preferred.
Aravind Machiry Oct. 4, 2020, 12:36 a.m. UTC | #4
Hi Luiz,

Yes. Although bt_malloc internally uses malloc, there are a couple of
places where bt_malloc is used instead of malloc.

I was sticking to the convention of replacing malloc with malloc0 and
bt_malloc with btmalloc0. I am not sure about any underlying reason
for using malloc vs bt_malloc.

If you think that bt_malloc/bt_malloc0 is the right way to go? I can
go ahead and replace all occurrences of malloc/malloc0 with
bt_malloc/bt_malloc0 respectively.

Please do let me know.

Btw, all the tests seem to pass when I did the replacement.

-Best,
Aravind




On Sat, Oct 3, 2020 at 2:49 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Hi Aravind,
>
> On Sat, Oct 3, 2020 at 11:14 AM Aravind Machiry <machiry@cs.ucsb.edu> wrote:
> >
> > Gentle reminder!
> >
> > On Mon, Sep 28, 2020 at 12:30 PM <bluez.test.bot@gmail.com> wrote:
> > >
> > > 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=356715
> > >
> > > ---Test result---
> > >
> > > ##############################
> > > Test: CheckPatch - PASS
> > >
> > > ##############################
> > > Test: CheckGitLint - PASS
> > >
> > > ##############################
> > > Test: CheckBuild - PASS
> > >
> > > ##############################
> > > Test: MakeCheck - PASS
> > >
> > >
> > >
> > > ---
> > > Regards,
> > > Linux Bluetooth
>
> There seems to be a mixture of malloc0 or bt_malloc0 when I guess the
> later should be preferred.
>
>
>
> --
> Luiz Augusto von Dentz
Luiz Augusto von Dentz Oct. 8, 2020, 12:01 a.m. UTC | #5
Hi Aravind,

On Sat, Oct 3, 2020 at 5:36 PM Aravind Machiry <machiry@cs.ucsb.edu> wrote:
>
> Hi Luiz,
>
> Yes. Although bt_malloc internally uses malloc, there are a couple of
> places where bt_malloc is used instead of malloc.
>
> I was sticking to the convention of replacing malloc with malloc0 and
> bt_malloc with btmalloc0. I am not sure about any underlying reason
> for using malloc vs bt_malloc.
>
> If you think that bt_malloc/bt_malloc0 is the right way to go? I can
> go ahead and replace all occurrences of malloc/malloc0 with
> bt_malloc/bt_malloc0 respectively.
>
> Please do let me know.
>
> Btw, all the tests seem to pass when I did the replacement.

Applied, thanks, note that I did replace the instances of malloc0 with
bt_malloc0.

> -Best,
> Aravind
>
>
>
>
> On Sat, Oct 3, 2020 at 2:49 PM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> >
> > Hi Aravind,
> >
> > On Sat, Oct 3, 2020 at 11:14 AM Aravind Machiry <machiry@cs.ucsb.edu> wrote:
> > >
> > > Gentle reminder!
> > >
> > > On Mon, Sep 28, 2020 at 12:30 PM <bluez.test.bot@gmail.com> wrote:
> > > >
> > > > 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=356715
> > > >
> > > > ---Test result---
> > > >
> > > > ##############################
> > > > Test: CheckPatch - PASS
> > > >
> > > > ##############################
> > > > Test: CheckGitLint - PASS
> > > >
> > > > ##############################
> > > > Test: CheckBuild - PASS
> > > >
> > > > ##############################
> > > > Test: MakeCheck - PASS
> > > >
> > > >
> > > >
> > > > ---
> > > > Regards,
> > > > Linux Bluetooth
> >
> > There seems to be a mixture of malloc0 or bt_malloc0 when I guess the
> > later should be preferred.
> >
> >
> >
> > --
> > Luiz Augusto von Dentz
diff mbox series

Patch

diff --git a/lib/bluetooth.c b/lib/bluetooth.c
index 0aecb50e1..84e40c819 100644
--- a/lib/bluetooth.c
+++ b/lib/bluetooth.c
@@ -173,6 +173,11 @@  void *bt_malloc(size_t size)
 	return malloc(size);
 }
 
+void *bt_malloc0(size_t size)
+{
+	return calloc(size, 1);
+}
+
 void bt_free(void *ptr)
 {
 	free(ptr);
diff --git a/lib/bluetooth.h b/lib/bluetooth.h
index 1619f5f08..6994c037a 100644
--- a/lib/bluetooth.h
+++ b/lib/bluetooth.h
@@ -149,6 +149,8 @@  enum {
 	BT_CLOSED
 };
 
+#define malloc0(n) (calloc((n), 1))
+
 /* Byte order conversions */
 #if __BYTE_ORDER == __LITTLE_ENDIAN
 #define htobs(d)  (d)
@@ -349,6 +351,7 @@  int basprintf(char *str, const char *format, ...);
 int basnprintf(char *str, size_t size, const char *format, ...);
 
 void *bt_malloc(size_t size);
+void *bt_malloc0(size_t size);
 void bt_free(void *ptr);
 
 int bt_error(uint16_t code);
diff --git a/lib/sdp.c b/lib/sdp.c
index a27cd3a7b..98624dcfc 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -345,12 +345,11 @@  sdp_data_t *sdp_data_alloc_with_length(uint8_t dtd, const void *value,
 							uint32_t length)
 {
 	sdp_data_t *seq;
-	sdp_data_t *d = malloc(sizeof(sdp_data_t));
+	sdp_data_t *d = malloc0(sizeof(sdp_data_t));
 
 	if (!d)
 		return NULL;
 
-	memset(d, 0, sizeof(sdp_data_t));
 	d->dtd = dtd;
 	d->unitSize = sizeof(uint8_t);
 
@@ -906,11 +905,10 @@  int sdp_gen_record_pdu(const sdp_record_t *rec, sdp_buf_t *buf)
 	memset(buf, 0, sizeof(sdp_buf_t));
 	sdp_list_foreach(rec->attrlist, sdp_attr_size, buf);
 
-	buf->data = malloc(buf->buf_size);
+	buf->data = malloc0(buf->buf_size);
 	if (!buf->data)
 		return -ENOMEM;
 	buf->data_size = 0;
-	memset(buf->data, 0, buf->buf_size);
 
 	sdp_list_foreach(rec->attrlist, sdp_attr_pdu, buf);
 
@@ -1030,12 +1028,11 @@  static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 		return NULL;
 	}
 
-	d = malloc(sizeof(sdp_data_t));
+	d = malloc0(sizeof(sdp_data_t));
 	if (!d)
 		return NULL;
 
 	SDPDBG("Extracting integer");
-	memset(d, 0, sizeof(sdp_data_t));
 	d->dtd = *(uint8_t *) p;
 	p += sizeof(uint8_t);
 	*len += sizeof(uint8_t);
@@ -1105,13 +1102,12 @@  static sdp_data_t *extract_int(const void *p, int bufsize, int *len)
 static sdp_data_t *extract_uuid(const uint8_t *p, int bufsize, int *len,
 							sdp_record_t *rec)
 {
-	sdp_data_t *d = malloc(sizeof(sdp_data_t));
+	sdp_data_t *d = malloc0(sizeof(sdp_data_t));
 
 	if (!d)
 		return NULL;
 
 	SDPDBG("Extracting UUID");
-	memset(d, 0, sizeof(sdp_data_t));
 	if (sdp_uuid_extract(p, bufsize, &d->val.uuid, len) < 0) {
 		free(d);
 		return NULL;
@@ -1136,11 +1132,10 @@  static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
 		return NULL;
 	}
 
-	d = malloc(sizeof(sdp_data_t));
+	d = malloc0(sizeof(sdp_data_t));
 	if (!d)
 		return NULL;
 
-	memset(d, 0, sizeof(sdp_data_t));
 	d->dtd = *(uint8_t *) p;
 	p += sizeof(uint8_t);
 	*len += sizeof(uint8_t);
@@ -1183,13 +1178,12 @@  static sdp_data_t *extract_str(const void *p, int bufsize, int *len)
 		return NULL;
 	}
 
-	s = malloc(n + 1);
+	s = malloc0(n + 1);
 	if (!s) {
 		SDPERR("Not enough memory for incoming string");
 		free(d);
 		return NULL;
 	}
-	memset(s, 0, n + 1);
 	memcpy(s, p, n);
 
 	*len += n;
@@ -1260,13 +1254,12 @@  static sdp_data_t *extract_seq(const void *p, int bufsize, int *len,
 {
 	int seqlen, n = 0;
 	sdp_data_t *curr, *prev;
-	sdp_data_t *d = malloc(sizeof(sdp_data_t));
+	sdp_data_t *d = malloc0(sizeof(sdp_data_t));
 
 	if (!d)
 		return NULL;
 
 	SDPDBG("Extracting SEQ");
-	memset(d, 0, sizeof(sdp_data_t));
 	*len = sdp_extract_seqtype(p, bufsize, &d->dtd, &seqlen);
 	SDPDBG("Sequence Type : 0x%x length : 0x%x", d->dtd, seqlen);
 
@@ -2740,12 +2733,11 @@  void sdp_uuid32_to_uuid128(uuid_t *uuid128, const uuid_t *uuid32)
 
 uuid_t *sdp_uuid_to_uuid128(const uuid_t *uuid)
 {
-	uuid_t *uuid128 = bt_malloc(sizeof(uuid_t));
+	uuid_t *uuid128 = bt_malloc0(sizeof(uuid_t));
 
 	if (!uuid128)
 		return NULL;
 
-	memset(uuid128, 0, sizeof(uuid_t));
 	switch (uuid->type) {
 	case SDP_UUID128:
 		*uuid128 = *uuid;
@@ -3191,12 +3183,11 @@  int sdp_record_update(sdp_session_t *session, const sdp_record_t *rec)
 
 sdp_record_t *sdp_record_alloc(void)
 {
-	sdp_record_t *rec = malloc(sizeof(sdp_record_t));
+	sdp_record_t *rec = malloc0(sizeof(sdp_record_t));
 
 	if (!rec)
 		return NULL;
 
-	memset(rec, 0, sizeof(sdp_record_t));
 	rec->handle = 0xffffffff;
 	return rec;
 }
@@ -3731,23 +3722,21 @@  sdp_session_t *sdp_create(int sk, uint32_t flags)
 	sdp_session_t *session;
 	struct sdp_transaction *t;
 
-	session = malloc(sizeof(sdp_session_t));
+	session = malloc0(sizeof(sdp_session_t));
 	if (!session) {
 		errno = ENOMEM;
 		return NULL;
 	}
-	memset(session, 0, sizeof(*session));
 
 	session->flags = flags;
 	session->sock = sk;
 
-	t = malloc(sizeof(struct sdp_transaction));
+	t = malloc0(sizeof(struct sdp_transaction));
 	if (!t) {
 		errno = ENOMEM;
 		free(session);
 		return NULL;
 	}
-	memset(t, 0, sizeof(*t));
 
 	session->priv = t;
 
@@ -4173,13 +4162,12 @@  int sdp_process(sdp_session_t *session)
 		return -1;
 	}
 
-	rspbuf = malloc(SDP_RSP_BUFFER_SIZE);
+	rspbuf = malloc0(SDP_RSP_BUFFER_SIZE);
 	if (!rspbuf) {
 		SDPERR("Response buffer alloc failure:%m (%d)", errno);
 		return -1;
 	}
 
-	memset(rspbuf, 0, SDP_RSP_BUFFER_SIZE);
 
 	t = session->priv;
 	reqhdr = (sdp_pdu_hdr_t *)t->reqbuf;