* Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx>:On Tuesday, August 19, 2008 11:39 am Alex Chiang wrote:* Rolf Eike Beer <eike-kernel@xxxxxxxxx>:If they're all identical, maybe we should pull this up into aAlex Chiang wrote:You're correct that we don't exactly need it in acpiphp. However,+static inline const char *slot_name(struct slot *slot)I don't see a point in this function. Why not call hotplug_slot_name()
+{
+ return hotplug_slot_name(slot->hotplug_slot);
+}
+
/*
* struct acpiphp_bridge - PCI bridge information
*
directly?
it is a useful helper function for some of the other drivers, and
I thought it would be better to keep consistency if possible.
Also, it helps later on, when trying to stay below the 80 column
limit. :)
common function (with a short name :). That can be a
subsequent cleanup though.
Ok, I'll save this for next time. Or someone else can have the
git glory. ;)
Yeah the main reason for separating whitespace changes fromYes, it's fuzz, but my practice has been to clean up* source files@@ -84,7 +87,6 @@ static struct hotplug_slot_ops acpi_hotplug_slot_opsFuzz.
= { .get_adapter_status = get_adapter_status,
};
-
/**
* acpiphp_register_attention - set attention LED callback
* @info: must be completely filled with LED callbacks
during the course of making actual, functional changes. Better
than sending a mostly-useless whitespace patchbomb, IMO.
* Note that "clean up" here means "reasonable cleanup" that
doesn't detract from reading the rest of the patch.
real ones is to avoid making the patch hard to read in case a
problem crops up. Killing an extra newline doesn't make things
harder to read, imo, so it's fine with me if you keep it
(though if you *do* end up doing a cleanup patch later you
could save it for that).
I'll keep the 'kill the newline' here but drop the kerneldoc
stuff. As you point out, kerneldoc will take either */ or **/.
Yeah, but you'll overwrite it with snprintf anyway, so the memset is redundant.Hm, don't need a memset? I won't have garbage on the stack?acpiphp_slot->slot = slot;The memset() is not needed at all. And the sizeof is IMHO a good idea
- snprintf(slot->name, sizeof(slot->name), "%u", slot->acpi_slot->sun);
+ memset(name, 0, SLOT_NAME_SIZE);
+ snprintf(name, SLOT_NAME_SIZE, "%u", slot->acpi_slot->sun);
anyway as it allows to get rid of the define.
</n00b>
Ok. I went through and removed the superfluous memsets, and
changed instances of snprintf() to scnprintf().
On the other hand, keeping the #define is important, becauseGoing from the define to sizeof() everywhere could be another
again, that's the established convention of the PCI hotplug
drivers.
Thanks for the review.
cleanup, but I don't have strong feelings about that.
Ok, saving these for next time too.
I'll hold off from sending v2 until I hear comments from
Kenji-san, so if he has any suggestions, I can incorporate them
all in v2.