From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:34640) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Un0BP-00053k-BK for qemu-devel@nongnu.org; Thu, 13 Jun 2013 01:34:52 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Un0BO-0007eX-9m for qemu-devel@nongnu.org; Thu, 13 Jun 2013 01:34:51 -0400 Received: from e28smtp05.in.ibm.com ([122.248.162.5]:59618) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Un0BN-0007dx-IT for qemu-devel@nongnu.org; Thu, 13 Jun 2013 01:34:50 -0400 Received: from /spool/local by e28smtp05.in.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Thu, 13 Jun 2013 11:00:01 +0530 Received: from d28relay02.in.ibm.com (d28relay02.in.ibm.com [9.184.220.59]) by d28dlp03.in.ibm.com (Postfix) with ESMTP id C5BFB1258051 for ; Thu, 13 Jun 2013 11:03:37 +0530 (IST) Received: from d28av01.in.ibm.com (d28av01.in.ibm.com [9.184.220.63]) by d28relay02.in.ibm.com (8.13.8/8.13.8/NCO v10.0) with ESMTP id r5D5Yh6k26017836 for ; Thu, 13 Jun 2013 11:04:44 +0530 Received: from d28av01.in.ibm.com (loopback [127.0.0.1]) by d28av01.in.ibm.com (8.14.4/8.13.1/NCO v10.0 AVout) with ESMTP id r5D5YfQU015120 for ; Thu, 13 Jun 2013 05:34:41 GMT Message-ID: <51B959A9.1080709@linux.vnet.ibm.com> Date: Thu, 13 Jun 2013 13:33:29 +0800 From: Wenchao Xia MIME-Version: 1.0 References: <1370674687-13849-1-git-send-email-xiawenc@linux.vnet.ibm.com> <1370674687-13849-6-git-send-email-xiawenc@linux.vnet.ibm.com> <20130611091405.GF18312@stefanha-thinkpad.redhat.com> In-Reply-To: <20130611091405.GF18312@stefanha-thinkpad.redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit Subject: Re: [Qemu-devel] [PATCH 05/11] snapshot: add paired functions for internal snapshot id and name List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi Cc: kwolf@redhat.com, phrdina@redhat.com, armbru@redhat.com, qemu-devel@nongnu.org, lcapitulino@redhat.com, stefanha@redhat.com, pbonzini@redhat.com, dietmar@proxmox.com 于 2013-6-11 17:14, Stefan Hajnoczi 写道: > On Sat, Jun 08, 2013 at 02:58:01PM +0800, Wenchao Xia wrote: >> +/* >> + * Every internal snapshot have an ID used by qemu block layer, this function >> + * check whether name used by user mess up with ID. An empty string is also >> + * invalid. >> + */ >> +bool snapshot_name_wellformed(const char *name) >> +{ >> + char *p; >> + /* variable v is used to remove gcc warning of "ignoring return value" and >> + "set but not used" */ >> + unsigned long v; >> + >> + if (*name == '\0') { >> + return false; >> + } >> + >> + v = strtoul(name, &p, 10); >> + v++; >> + >> + if (*p == '\0') { >> + /* Numeric string */ >> + return false; >> + } >> + return true; >> +} > > Shorter function with the same behavior and a rewritten doc comment: > > /* > * Return true if the given internal snapshot name is valid, false > * otherwise. > * > * To prevent clashes with internal snapshot IDs, names consisting only > * of digits are rejected. Empty strings are also rejected. > */ > bool snapshot_name_wellformed(const char *name) > { > return strspn(name, "0123456789") != strlen(name); > } > much nicer, will use it, thanks! >> + >> +/* Following function generate id string, used by block driver, such as qcow2. >> + Since no better place to go, place the funtion here for now. */ >> +void snapshot_id_string_generate(int id, char *id_str, int id_str_size) >> +{ >> + snprintf(id_str, id_str_size, "%d", id); >> +} > > Since the caller has to manage id, this function doesn't really abstract > anything. I would keep the snprintf() inline, there's only one caller. > Its purpose is move the id rules from one implemention(qcow2) into generic. If not, it would be a question why snapshot_name_wellformed() could make sure name not conflict with ID, then reader find the answer in qcow2... I think at least a document is needed about how should implemention under ./block generate snapshot ID. -- Best Regards Wenchao Xia