From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37887) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDsyE-0001xY-JS for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:57:11 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YDsy8-0003bv-3B for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:57:10 -0500 Received: from static.88-198-71-155.clients.your-server.de ([88.198.71.155]:39992 helo=socrates.bennee.com) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YDsy7-0003bn-S1 for qemu-devel@nongnu.org; Wed, 21 Jan 2015 05:57:03 -0500 References: <1421706621-23731-1-git-send-email-greg.bellows@linaro.org> <1421706621-23731-2-git-send-email-greg.bellows@linaro.org> <8738758rgf.fsf@linaro.org> From: Alex =?utf-8?Q?Benn=C3=A9e?= In-reply-to: Date: Wed, 21 Jan 2015 10:57:04 +0000 Message-ID: <87vbk07667.fsf@linaro.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 1/5] target-arm: Add ARM CPU feature parsing List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Greg Bellows Cc: Peter Maydell , QEMU Developers , Christoffer Dall Greg Bellows writes: > Thanks Alex comments inline.... > >> >> Aren't we leaking here? strtok returns the next token (or NULL) so don't >> we loose the original ptr? >> >> > ​As I understand it, strtok uses static pointers to track the location > within an existing string rather than allocating storage that the user must > free. This is apparently what makes the version I used non-reentrant.​ In > which case, there should not be an leak due to its use. Yeah - I realised this after re-reading the man page. Non-re-entrant isn't a particular problem these days but it still feels dirty.... >> Also while using glib you might want to consider using glib's own >> tokenising functions (e.g. g_strsplit). This has the advantage of having >> helper functions like g_strfreev() which can clean-up everything in one go. >> > > ​I certainly can use the glib version, but in this case I did not see the > advantage. In fact, it actually may be less performant​ to use the glib > version given it needs to allocate/free memory. I am fine either way if > anyone feels strongly. I suspect this discussion is trumped by moving to the feat_foo=on/off parsing style referenced elsewhere so we can use common code. -- Alex Bennée