On 06/19/13 19:42, Ewan D. Milne wrote:
+static void starget_evt_emit(struct scsi_target *starget,
+                            struct starget_event *evt)
+{
+       int idx = 0;
+       char *envp[3];
+
+       switch (evt->evt_type) {
+       case STARGET_EVT_LUN_CHANGE_REPORTED:
+               envp[idx++] = "STARGET_LUN_CHANGE_REPORTED=1";
+               break;
+       default:
+               /* do nothing */
+               break;
+       }
+
+       envp[idx++] = NULL;
+
+       kobject_uevent_env(&starget->dev.kobj, KOBJ_CHANGE, envp);
+}

Sorry but it's not clear to me why the envp[] array has size three while at most two entries are used ? And shouldn't that array be declared as const char*[] instead of char *[] since string constants have type const char[] ?

+               list_for_each_safe(this, tmp, &event_list) {
+                       evt = list_entry(this, struct starget_event, node);

Any reason why list_for_each_entry_safe() has not been used here ?

+void starget_evt_send(struct scsi_target *starget, struct starget_event *evt)
+{
+       unsigned long flags;
+
+       spin_lock_irqsave(&starget->list_lock, flags);
+       list_add_tail(&evt->node, &starget->event_list);
+       schedule_work(&starget->event_work);
+       spin_unlock_irqrestore(&starget->list_lock, flags);
+}

Is it necessary here to invoke schedule_work() under protection of list_lock, or would it be safe to invoke schedule_work() after the spin_unlock_irqrestore() ?

+#ifdef CONFIG_SCSI_ENHANCED_UA
+       struct list_head *this, *tmp;
+
+       cancel_work_sync(&starget->event_work);
+
+       list_for_each_safe(this, tmp, &starget->event_list) {
+               struct starget_event *evt;

+               evt = list_entry(this, struct starget_event, node);
+               list_del(&evt->node);
+               kfree(evt);
+       }
+#endif

Same question here: any reason why list_for_each_entry_safe() has not been used ?

Thanks,

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to