Return-Path: From: "Gix, Brian" To: Felipe Ferreri Tonello , "linux-bluetooth@vger.kernel.org" CC: "Von Dentz, Luiz" , Szymon Janc Subject: RE: [PATCH v5 2/2] Make unit test compatible with GLIB v2.28 Date: Wed, 25 Jan 2017 16:43:19 +0000 Message-ID: References: <1485287564-16879-1-git-send-email-brian.gix@intel.com> <1485287564-16879-3-git-send-email-brian.gix@intel.com> <6feed7de-27f4-f484-e851-1e5a459e154b@felipetonello.com> In-Reply-To: <6feed7de-27f4-f484-e851-1e5a459e154b@felipetonello.com> Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Sender: linux-bluetooth-owner@vger.kernel.org List-ID: Hi Felipe, On 25/1/17 Felipe Ferreri Tonello wrote: > > Hi Brian, > > On 24/01/17 20:10, Gix, Brian wrote: > > > > H Luiz, Felipe, Szymon, > > > >> From: Gix, Brian > >> > >> --- > >> unit/test-midi.c | 8 ++++++-- > >> 1 file changed, 6 insertions(+), 2 deletions(-) > >> > >> diff --git a/unit/test-midi.c b/unit/test-midi.c index > >> d318b07..3995c86 100644 > >> --- a/unit/test-midi.c > >> +++ b/unit/test-midi.c > >> @@ -282,8 +282,12 @@ static void compare_events(const > snd_seq_event_t > >> *ev1, > >> ev2->data.control.value); > >> break; > >> case SND_SEQ_EVENT_SYSEX: > >> - g_assert_cmpmem(ev1->data.ext.ptr, ev1->data.ext.len, > >> - ev2->data.ext.ptr, ev2->data.ext.len); > >> + g_assert_cmpint(ev1->data.ext.len, > >> + ==, > >> + ev2->data.ext.len); > >> + g_assert(memcmp(ev1->data.ext.ptr, > >> + ev2->data.ext.ptr, > >> + ev2->data.ext.len) == 0); > >> break; > >> default: > >> g_assert_not_reached(); > > > > > > Here is a straightforward rework of the g_assert_cmpmem assert. > > > > It was used only once, and both replacement asserts existed pre-v2.28 > > > > Fine by me, but make sure you add a relevant comment in the git message, > please. > > Just the title is not enough. You could write something like: > > g_assert_cmpmem was added in version X of glib and since we don't want to > bump the required version of glib for BlueZ because of a unit-test, we use > g_assert_cmpint and g_assert to replace previous code. I can put this in the comment if people like, but it is a very straightforward change. You don't think the existing comment makes it clear that the change is being done to avoid up-reving GLIB? It is a *very* isolated change that affects only asserts, for the reason given in the headline. I think more verbiage would be excessive. --Brian