All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hollis Blanchard <hollisb@us.ibm.com>
To: jyoung5@us.ibm.com
Cc: kvm-devel@lists.sourceforge.net, kvm-ppc-devel@lists.sourceforge.net
Subject: Re: [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic
Date: Tue, 18 Mar 2008 22:08:14 +0000	[thread overview]
Message-ID: <1205878094.11784.51.camel@basalt> (raw)
In-Reply-To: <1205876152.23113.18.camel@thinkpad.austin.ibm.com>

I can only assume that you will actually make the corrections that you
didn't respond to in this mail.

On Tue, 2008-03-18 at 16:35 -0500, Jerone Young wrote:
> On Tue, 2008-03-18 at 16:25 -0500, Hollis Blanchard wrote:
> > 
> > > +#define DT_PROC_INTERFACE_PATH "/proc/device-tree"
> > > +
> > > +/* FUNCTIONS FOR READING FROM DEVICE TREE OF HOST IN /PROC */
> > > +
> > > +/* This function reads device-tree property files that are of
> > > + * a single cell size
> > > + */
> > > +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree)
> > > +{
> > > +	char *buf = NULL;
> > > +	int i;
> > > +	uint32_t num;
> > > +	FILE *stream;
> > > +
> > > +	i = snprintf(buf, 0, "%s/%s", DT_PROC_INTERFACE_PATH,
> > > +		path_in_device_tree);
> > > +
> > > +	buf = (char *)malloc(i);
> > > +	if (buf = NULL)
> > > +	{
> > > +		printf("%s: Unable to malloc string buffer buf\n",
> > > +			__func__);
> > > +		exit(1);
> > > +	}
> > 
> > Braces.
> 
> What is the deal. They are braces. They are done diffrenent through outt
> the qemu code. This

I don't enjoy correcting whitespace, and in fact I hate it when that's
all comments are about. If it weren't so egregious in this patch series,
I probably would have let it slide.

In general I don't really care as long as it's *consistent*. The tabs
you use in this code clashes with the rest of qemu, and that makes it
difficult to use the right editor settings. Despite that, I still didn't
say anything, because at least it's consistent.

In general, don't just make your code work; make it pretty too.

> > > @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in
> > >  			const char *initrd_filename,
> > >  			const char *cpu_model)
> > >  {
> > > +	char buf[1024];
> > 
> > You previously said you had removed 'buf' and replaced it with dynamic
> > allocation, but I don't see that here.
> 
> Removing of buf discussed was from hw/device_tree.c  not this file.

So you can fix it here too, right?

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
_______________________________________________
kvm-ppc-devel mailing list
kvm-ppc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/kvm-ppc-devel

WARNING: multiple messages have this Message-ID (diff)
From: Hollis Blanchard <hollisb@us.ibm.com>
To: jyoung5@us.ibm.com
Cc: kvm-devel@lists.sourceforge.net, kvm-ppc-devel@lists.sourceforge.net
Subject: Re: [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device	tree	manipulation & change uboot	loader for PPC bamboo board model
Date: Tue, 18 Mar 2008 17:08:14 -0500	[thread overview]
Message-ID: <1205878094.11784.51.camel@basalt> (raw)
In-Reply-To: <1205876152.23113.18.camel@thinkpad.austin.ibm.com>

I can only assume that you will actually make the corrections that you
didn't respond to in this mail.

On Tue, 2008-03-18 at 16:35 -0500, Jerone Young wrote:
> On Tue, 2008-03-18 at 16:25 -0500, Hollis Blanchard wrote:
> > 
> > > +#define DT_PROC_INTERFACE_PATH "/proc/device-tree"
> > > +
> > > +/* FUNCTIONS FOR READING FROM DEVICE TREE OF HOST IN /PROC */
> > > +
> > > +/* This function reads device-tree property files that are of
> > > + * a single cell size
> > > + */
> > > +uint32_t read_proc_dt_prop_cell(char *path_in_device_tree)
> > > +{
> > > +	char *buf = NULL;
> > > +	int i;
> > > +	uint32_t num;
> > > +	FILE *stream;
> > > +
> > > +	i = snprintf(buf, 0, "%s/%s", DT_PROC_INTERFACE_PATH,
> > > +		path_in_device_tree);
> > > +
> > > +	buf = (char *)malloc(i);
> > > +	if (buf == NULL)
> > > +	{
> > > +		printf("%s: Unable to malloc string buffer buf\n",
> > > +			__func__);
> > > +		exit(1);
> > > +	}
> > 
> > Braces.
> 
> What is the deal. They are braces. They are done diffrenent through outt
> the qemu code. This

I don't enjoy correcting whitespace, and in fact I hate it when that's
all comments are about. If it weren't so egregious in this patch series,
I probably would have let it slide.

In general I don't really care as long as it's *consistent*. The tabs
you use in this code clashes with the rest of qemu, and that makes it
difficult to use the right editor settings. Despite that, I still didn't
say anything, because at least it's consistent.

In general, don't just make your code work; make it pretty too.

> > > @@ -26,14 +27,22 @@ void bamboo_init(ram_addr_t ram_size, in
> > >  			const char *initrd_filename,
> > >  			const char *cpu_model)
> > >  {
> > > +	char buf[1024];
> > 
> > You previously said you had removed 'buf' and replaced it with dynamic
> > allocation, but I don't see that here.
> 
> Removing of buf discussed was from hw/device_tree.c  not this file.

So you can fix it here too, right?

-- 
Hollis Blanchard
IBM Linux Technology Center


-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

  reply	other threads:[~2008-03-18 22:08 UTC|newest]

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-03-18 20:06 [kvm-ppc-devel] [PATCH 0 of 7] [v3] PowerPC kvm-userspace patches Jerone Young
2008-03-18 20:06 ` Jerone Young
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 1 of 7] Add libfdt to KVM userspace Jerone Young
2008-03-18 20:06   ` Jerone Young
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 2 of 7] Add libfdt support to qemu Jerone Young
2008-03-18 20:06   ` Jerone Young
2008-03-18 21:16   ` [kvm-ppc-devel] " Hollis Blanchard
2008-03-18 21:16     ` Hollis Blanchard
2008-03-18 21:22     ` Jerone Young
2008-03-18 21:22       ` Jerone Young
2008-03-18 21:28       ` Hollis Blanchard
2008-03-18 21:28         ` Hollis Blanchard
2008-03-18 21:57         ` [kvm-ppc-devel] [kvm-devel] [PATCH 2 of 7] Add libfdt Jerone Young
2008-03-18 21:57           ` [kvm-ppc-devel] [PATCH 2 of 7] Add libfdt support to qemu Jerone Young
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() & gunzip Jerone Young
2008-03-18 20:06   ` [PATCH 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu Jerone Young
2008-03-18 21:14   ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() Hollis Blanchard
2008-03-18 21:14     ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu Hollis Blanchard
2008-03-18 21:46     ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() Jerone Young
2008-03-18 21:46       ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu Jerone Young
2008-03-18 22:14       ` [kvm-ppc-devel] [PATCH 3 of 7] Create new Hollis Blanchard
2008-03-18 22:14         ` [kvm-ppc-devel] [PATCH 3 of 7] Create new load_uimage() & gunzip support to uboot loader in Qemu Hollis Blanchard
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 4 of 7] Add PPC 440EP bamboo board device Jerone Young
2008-03-18 20:06   ` [PATCH 4 of 7] Add PPC 440EP bamboo board device tree source & binary into qemu Jerone Young
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation Jerone Young
2008-03-18 20:06   ` [PATCH 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model Jerone Young
2008-03-18 21:25   ` [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device Hollis Blanchard
2008-03-18 21:25     ` [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model Hollis Blanchard
2008-03-18 21:35     ` [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device Jerone Young
2008-03-18 21:35       ` [kvm-ppc-devel] [PATCH 5 of 7] Add dynamic device tree manipulation & change uboot loader for PPC bamboo board model Jerone Young
2008-03-18 22:08       ` Hollis Blanchard [this message]
2008-03-18 22:08         ` Hollis Blanchard
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 6 of 7] Modify PPC bamboo & ppc440 board Jerone Young
2008-03-18 20:06   ` [PATCH 6 of 7] Modify PPC bamboo & ppc440 board models Jerone Young
2008-03-18 20:06 ` [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on Jerone Young
2008-03-18 20:06   ` [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model Jerone Young
2008-03-18 21:03   ` [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram Hollis Blanchard
2008-03-18 21:03     ` [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model Hollis Blanchard
2008-03-18 21:17     ` [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram Jerone Young
2008-03-18 21:17       ` [kvm-ppc-devel] [PATCH 7 of 7] Add ability to specify ram on command line for bamboo board model Jerone Young

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1205878094.11784.51.camel@basalt \
    --to=hollisb@us.ibm.com \
    --cc=jyoung5@us.ibm.com \
    --cc=kvm-devel@lists.sourceforge.net \
    --cc=kvm-ppc-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.