Re: [PATCH] efivars update
From: Matt Domsch
Date: Fri Aug 29 2003 - 17:19:02 EST
> The patch is against 2.6.0-test4. Please apply.
Thanks for your work on this! One bug, 3 nits...
> +static ssize_t
> +efivar_edit(struct efivar_entry *entry, const char *buf, size_t count)
...
> + spin_lock(&efivars_lock);
> + status = efi.set_variable(new_var->VariableName,
> + &new_var->VendorGuid,
> + new_var->Attributes,
> + new_var->DataSize,
> + new_var->Data);
> +
> + if (status != EFI_SUCCESS) {
> + printk(KERN_WARNING "set_variable() failed: status=%lx\n",
> + status);
> + return -EIO;
> + }
> + spin_unlock(&efivars_lock);
Move the unlock up above the status test, else you return while
holding the lock on failure.
> +static ssize_t
> +efivar_create(struct subsystem *sub, const char *buf, size_t count)
...
> + if (found) {
> + printk("EFI variable already exists!\n");
level on printk please.
> +efivar_delete(struct subsystem *sub, const char *buf, size_t count)
...
> + status = efi.set_variable(del_var->VariableName,
> + &del_var->VendorGuid,
> + del_var->Attributes,
> + del_var->DataSize,
> + del_var->Data);
Might as well just force the deletion by setting Attributes=0 or
DataSize=0 or both. Less chance for userspace error.
> + * Overrides for Emacs so that we follow Linus's tabbing style.
Please kill this chunk. I shouldn't have had it there in the first
place. :-)
Looks great to me! Thanks again.
--
Matt Domsch
Sr. Software Engineer, Lead Engineer
Dell Linux Solutions www.dell.com/linux
Linux on Dell mailing lists @ http://lists.us.dell.com
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/