Skip to contentSkip to conversation

EVMS core (3/9) discover.c

PATCH Andi Kleen 2 participants 2 messages Oct 10, 2002 2 messages not in the archive

lore
Share & tools
lore

Andi Kleen

author
#2 lore raw
In reply to a message not in the archive

Review

Quoted 6 lines · Kevin Corry
(quoted diff) +6 -0
+	list_for_each_entry(plugin, &plugin_head, headers) {
+		if (GetPluginType(plugin->id) == EVMS_DEVICE_MANAGER) {
+			spin_unlock(&plugin_lock);
+			DISCOVER(plugin, disk_list);
+			spin_lock(&plugin_lock);
+		}

How do you know "plugin" and its successors are still valid when retaking the spinlock? Looks like you need a reference count on the object here.

Similar with other functions.

Review

Quoted 4 lines
(quoted diff) +4 -0
+
+	if (!gd) {
+		gd = alloc_disk();
+		BUG_ON(!gd);

BUG_ON ? Can't this fail for legal reasons?

-Andi

#3 lore raw
Quoted 7 lines · Andi Kleen

Review

Quoted 4 lines · Kevin Corry
(quoted diff) +4 -0
+
+	if (!gd) {
+		gd = alloc_disk();
+		BUG_ON(!gd);

BUG_ON ? Can't this fail for legal reasons?

Yes, it can. This, and a couple other incorrect BUG_ON() statements have been fixed to fail gracefully. Thanks for catching this.

Signature · 3 lines
Kevin Corry
[email protected]
http://evms.sourceforge.net/