From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=51624 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PHtfh-0000Q3-Sg
	for qemu-devel@nongnu.org; Mon, 15 Nov 2010 02:40:15 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <gleb@redhat.com>) id 1PHtfg-0004CA-OL
	for qemu-devel@nongnu.org; Mon, 15 Nov 2010 02:40:13 -0500
Received: from mx1.redhat.com ([209.132.183.28]:52289)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <gleb@redhat.com>) id 1PHtfg-0004C1-Gz
	for qemu-devel@nongnu.org; Mon, 15 Nov 2010 02:40:12 -0500
Date: Mon, 15 Nov 2010 09:40:08 +0200
From: Gleb Natapov <gleb@redhat.com>
Message-ID: <20101115074008.GF7948@redhat.com>
References: <1289749181-12070-1-git-send-email-gleb@redhat.com>
	<1289749181-12070-16-git-send-email-gleb@redhat.com>
	<20101115034033.GA1309@morn.localdomain>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20101115034033.GA1309@morn.localdomain>
Subject: [Qemu-devel] Re: [PATCHv4 15/15] Pass boot device list to firmware.
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Kevin O'Connor <kevin@koconnor.net>
Cc: kvm@vger.kernel.org, mst@redhat.com, qemu-devel@nongnu.org, armbru@redhat.com, blauwirbel@gmail.com, alex.williamson@redhat.com

On Sun, Nov 14, 2010 at 10:40:33PM -0500, Kevin O'Connor wrote:
> On Sun, Nov 14, 2010 at 05:39:41PM +0200, Gleb Natapov wrote:
> > +/*
> > + * This function returns device list as an array in a below format:
> > + * +-----+-----+---------------+-----+---------------+--
> > + * |  n  |  l1 |   devpath1    |  l2 |  devpath2     | ...
> > + * +-----+-----+---------------+-----+---------------+--
> > + * where:
> > + *   n - a number of devise pathes (one byte)
> > + *   l - length of following device path string (one byte)
> > + *   devpath - non-null terminated string of length l representing
> > + *             one device path
> > + */
> 
> Why not just return a newline separated list that is null terminated?
> 
Doing it like this will needlessly complicate firmware side. How do you
know how much memory to allocate before reading device list? Doing it
like Blue suggest (have BOOTINDEX_LEN and BOOTINDEX_STRING) solves this.
To create nice array from bootindex string you firmware will still have
to do additional pass on it though. With format like above the code
would look like that:

qemu_cfg_read(&n, 1);
arr = alloc(n);
for (i=0; i<n; i++) {
 qemu_cfg_read(&l, 1);
 arr[i] = zalloc(l+1);
 qemu_cfg_read(arr[i], l);
}
 

--
			Gleb.