From: Jeremy Fitzhardinge <jeremy@goop.org>
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
Cc: "xen-devel@lists.xensource.com" <xen-devel@lists.xensource.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [Xen-devel] [PATCH 2 of 5] early PV on HVM
Date: Fri, 12 Mar 2010 12:22:07 -0800 [thread overview]
Message-ID: <4B9AA26F.4000508@goop.org> (raw)
In-Reply-To: <alpine.DEB.2.00.1003121819500.27222@kaball-desktop>
On 03/12/2010 11:09 AM, Stefano Stabellini wrote:
> My version is not 64-bit specific, should I move it to a new file
> anyway?
>
> I was trying to keep the differences with Sheng's version low, but in
> this case I am not quite sure how to manage it.
> Suggestions are welcome.
>
My rough rules for code layout (in no particular order):
* keep similar functions together (even if they're not necessarily
sharing common code)
* dislike externs and cross-file references (ie, prefer static
functions/data) -> keep code sharing functions/data together
* dislike #ifdef CONFIG_x within files; prefer Makefile rules to
either completely compile or not compile files
* dislike dumping-ground files
* dislike too-small files
* dislike moving code between files, so its better to get it right
from the start (it makes merging harder)
So in this case, enlighten.c is the general setup/init/misc file in
xen/. I've been trying to move things out of it to reduce its dumping
ground-ness. In your case, the changes are clearly init, so they
probably belong there. They're compiled unconditionally, so there's no
#ifdef. They're fairly small so they would probably be too small in a
separate file (unless its likely to get bigger?). So keep in enlighten.c.
(In Sheng's version of this patch, it has more code and is conditionally
compiled, so I think the balance tips the other way.)
J
prev parent reply other threads:[~2010-03-12 20:22 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-03-10 15:47 [PATCH 2 of 5] early PV on HVM Stefano Stabellini
2010-03-11 21:20 ` [Xen-devel] " Jeremy Fitzhardinge
2010-03-12 19:09 ` Stefano Stabellini
2010-03-12 20:22 ` Jeremy Fitzhardinge [this message]
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=4B9AA26F.4000508@goop.org \
--to=jeremy@goop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=stefano.stabellini@eu.citrix.com \
--cc=xen-devel@lists.xensource.com \
/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.