From 1bc44d6c4fe0498f8ed2c52164f034ba483ccfb2 Mon Sep 17 00:00:00 2001
From: Peter Jones <pjones@redhat.com>
Date: Wed, 15 Oct 2014 10:35:56 -0400
Subject: [PATCH 20/31] Don't error on unset BootOrder when we're trying to add
to or rm from it.
Also print some better error messases here and there.
Signed-off-by: Peter Jones <pjones@redhat.com>
---
README | 1 -
src/efibootmgr/efibootmgr.c | 126 +++++++++++++++++++++++++++-----------------
src/lib/efi.c | 95 +++++++++++++++++++--------------
src/man/man8/efibootmgr.8 | 3 --
4 files changed, 131 insertions(+), 94 deletions(-)
diff --git a/README b/README
index 3bc0a26..edbce4b 100644
--- a/README
+++ b/README
@@ -29,7 +29,6 @@ usage: efibootmgr [options]
-O | --delete-bootorder delete BootOrder
-p | --part part (defaults to 1) containing loader
-q | --quiet be quiet
- --test filename don't write to NVRAM, write to filename
-t | --timeout seconds Boot manager timeout
-T | --delete-timeout delete Timeout value
-u | --unicode | --UCS-2 pass extra args as UCS-2 (default is ASCII)
diff --git a/src/efibootmgr/efibootmgr.c b/src/efibootmgr/efibootmgr.c
index 4d80f87..eb13942 100644
--- a/src/efibootmgr/efibootmgr.c
+++ b/src/efibootmgr/efibootmgr.c
@@ -33,6 +33,7 @@
#define _GNU_SOURCE
#include <ctype.h>
+#include <err.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -123,7 +124,7 @@ read_vars(char **namelist,
}
return;
err:
- fprintf(stderr, "efibootmgr: %m\n");
+ warn("efibootmgr");
exit(1);
}
@@ -242,16 +243,20 @@ make_boot_var(list_t *boot_list)
free_number = opts.bootnum;
}
- if (free_number == -1)
+ if (free_number == -1) {
+ warn("efibootmgr: no available boot variables");
return NULL;
+ }
/* Create a new efi_variable_t object
and populate it.
*/
boot = calloc(1, sizeof(*boot));
- if (!boot)
+ if (!boot) {
+ warn("efibootmgr");
return NULL;
+ }
if (make_linux_load_option(&boot->data, &boot->data_size) < 0)
goto err_boot_entry;
if (append_extra_args(&boot->data, &boot->data_size) < 0)
@@ -260,8 +265,10 @@ make_boot_var(list_t *boot_list)
boot->num = free_number;
boot->guid = EFI_GLOBAL_VARIABLE;
rc = asprintf(&boot->name, "Boot%04X", free_number);
- if (rc < 0)
+ if (rc < 0) {
+ warn("efibootmgr");
goto err_boot_entry;
+ }
boot->attributes = EFI_VARIABLE_NON_VOLATILE |
EFI_VARIABLE_BOOTSERVICE_ACCESS |
EFI_VARIABLE_RUNTIME_ACCESS;
@@ -272,8 +279,12 @@ make_boot_var(list_t *boot_list)
list_add_tail(&boot->list, boot_list);
return boot;
err_boot_entry:
- if (boot->name)
+ if (boot->name) {
+ warn("Could not set variable %s", boot->name);
free(boot->name);
+ } else {
+ warn("Could not set variable");
+ }
if (boot->data)
free(boot->data);
free(boot);
@@ -313,6 +324,15 @@ read_boot_order(efi_variable_t **boot_order)
}
static int
+set_boot_u16(const char *name, uint16_t num)
+{
+ return efi_set_variable(EFI_GLOBAL_GUID, name, (uint8_t *)&num,
+ sizeof (num), EFI_VARIABLE_NON_VOLATILE |
+ EFI_VARIABLE_BOOTSERVICE_ACCESS |
+ EFI_VARIABLE_RUNTIME_ACCESS);
+}
+
+static int
add_to_boot_order(uint16_t num)
{
efi_variable_t *boot_order = NULL;
@@ -321,8 +341,11 @@ add_to_boot_order(uint16_t num)
int rc;
rc = read_boot_order(&boot_order);
- if (rc < 0)
+ if (rc < 0) {
+ if (errno == ENOENT)
+ rc = set_boot_u16("BootOrder", num);
return rc;
+ }
/* We've now got an array (in boot_order->data) of the
* boot order. First add our entry, then copy the old array.
@@ -358,8 +381,11 @@ remove_dupes_from_boot_order(void)
int rc;
rc = read_boot_order(&boot_order);
- if (rc < 0)
+ if (rc < 0) {
+ if (errno == ENOENT)
+ rc = 0;
return rc;
+ }
old_data = (uint16_t *)(boot_order->data);
/* Start with the same size */
@@ -409,8 +435,11 @@ remove_from_boot_order(uint16_t num)
int rc;
rc = read_boot_order(&boot_order);
- if (rc < 0)
+ if (rc < 0) {
+ if (errno == ENOENT)
+ rc = 0;
return rc;
+ }
/* We've now got an array (in boot_order->data) of the
boot order. Simply copy the array, skipping the
@@ -470,15 +499,6 @@ read_boot_u16(const char *name)
}
static int
-set_boot_u16(const char *name, uint16_t num)
-{
- return efi_set_variable(EFI_GLOBAL_GUID, name, (uint8_t *)&num,
- sizeof (num), EFI_VARIABLE_NON_VOLATILE |
- EFI_VARIABLE_BOOTSERVICE_ACCESS |
- EFI_VARIABLE_RUNTIME_ACCESS);
-}
-
-static int
delete_boot_var(uint16_t num)
{
int rc;
@@ -490,13 +510,18 @@ delete_boot_var(uint16_t num)
rc = efi_del_variable(EFI_GLOBAL_GUID, name);
/* For backwards compatibility, try to delete abcdef entries as well */
- if (rc < 0 && errno == ENOENT) {
- snprintf(name, sizeof(name), "Boot%04x", num);
- rc = efi_del_variable(EFI_GLOBAL_GUID, name);
+ if (rc < 0) {
+ if (errno == ENOENT) {
+ snprintf(name, sizeof(name), "Boot%04x", num);
+ rc = efi_del_variable(EFI_GLOBAL_GUID, name);
+ } else if (errno == EPERM) {
+ warn("Could not delete Boot%04X", num);
+ return rc;
+ }
}
if (rc < 0) {
- fprintf(stderr,"\nboot entry: %X not found\n\n",num);
+ warnx("Boot entry %04X not found", num);
return rc;
}
list_for_each_safe(pos, n, &boot_entry_list) {
@@ -512,7 +537,6 @@ delete_boot_var(uint16_t num)
return 0;
}
-
static void
set_var_nums(list_t *list)
{
@@ -1177,34 +1201,28 @@ main(int argc, char **argv)
if (opts.iface && (
opts.acpi_hid < 0 || opts.acpi_uid < 0 ||
opts.acpi_hid > UINT32_MAX ||
- opts.acpi_uid > UINT32_MAX)) {
- fprintf(stderr, "\nYou must specify the ACPI HID and UID when using -i.\n\n");
- return 1;
- }
+ opts.acpi_uid > UINT32_MAX))
+ errx(1, "You must specify the ACPI HID and UID when using -i.");
- if (!efi_variables_supported()) {
- fprintf(stderr, "\nEFI variables are not supported on this system.\n\n");
- return 1;
- }
+ if (!efi_variables_supported())
+ errx(2, "EFI variables are not supported on this system.");
read_boot_var_names(&boot_names);
read_vars(boot_names, &boot_entry_list);
set_var_nums(&boot_entry_list);
if (opts.delete_boot) {
- if (opts.bootnum == -1) {
- fprintf(stderr, "\nYou must specify a boot entry to delete (see the -b option).\n\n");
- return 1;
- }
+ if (opts.bootnum == -1)
+ errx(3, "You must specify a boot entry to delete "
+ "(see the -b option).");
else
ret = delete_boot_var(opts.bootnum);
}
if (opts.active >= 0) {
- if (opts.bootnum == -1) {
- fprintf(stderr, "\nYou must specify a boot entry to activate (see the -b option).\n\n");
- return 1;
- }
+ if (opts.bootnum == -1)
+ errx(4, "You must specify a boot entry to activate "
+ "(see the -b option");
else
ret=set_active_state();
}
@@ -1212,47 +1230,57 @@ main(int argc, char **argv)
if (opts.create) {
warn_duplicate_name(&boot_entry_list);
new_boot = make_boot_var(&boot_entry_list);
- if (!new_boot) {
- fprintf(stderr, "\nCould not prepare boot variable: %m\n\n");
- return 1;
- }
+ if (!new_boot)
+ err(5, "Could not prepare boot variable");
/* Put this boot var in the right BootOrder */
if (new_boot)
ret=add_to_boot_order(new_boot->num);
+ if (ret)
+ err(6, "Could not add entry to BootOrder");
}
if (opts.delete_bootorder) {
ret = efi_del_variable(EFI_GLOBAL_GUID, "BootOrder");
+ err(7, "Could not remove entry from BootOrder");
}
if (opts.bootorder) {
ret = set_boot_order(opts.keep_old_entries);
+ if (ret)
+ err(8, "Could not set BootOrder");
}
if (opts.deduplicate) {
ret = remove_dupes_from_boot_order();
+ if (ret)
+ err(9, "Could not set BootOrder");
}
if (opts.delete_bootnext) {
ret = efi_del_variable(EFI_GLOBAL_GUID, "BootNext");
+ if (ret)
+ err(10, "Could not set BootNext");
}
if (opts.delete_timeout) {
ret = efi_del_variable(EFI_GLOBAL_GUID, "Timeout");
+ if (ret)
+ err(11, "Could not delete Timeout");
}
if (opts.bootnext >= 0) {
- if (!is_current_boot_entry(opts.bootnext & 0xFFFF)){
- fprintf (stderr,"\n\nboot entry %X does not exist\n\n",
- opts.bootnext);
- return 1;
- }
- ret=set_boot_u16("BootNext", opts.bootnext & 0xFFFF);
+ if (!is_current_boot_entry(opts.bootnext & 0xFFFF))
+ errx(12, "Boot entry %X does not exist", opts.bootnext);
+ ret = set_boot_u16("BootNext", opts.bootnext & 0xFFFF);
+ if (ret)
+ err(13, "Could not set BootNext");
}
if (opts.set_timeout) {
- ret=set_boot_u16("Timeout", opts.timeout);
+ ret = set_boot_u16("Timeout", opts.timeout);
+ if (ret)
+ err(14, "Could not set Timeout");
}
if (!opts.quiet && ret == 0) {
diff --git a/src/lib/efi.c b/src/lib/efi.c
index 7cdc884..d19c00d 100644
--- a/src/lib/efi.c
+++ b/src/lib/efi.c
@@ -19,6 +19,7 @@
*/
#include <ctype.h>
+#include <err.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
@@ -502,7 +503,6 @@ static ssize_t
make_disk_load_option(char *disk, uint8_t *buf, size_t size)
{
int disk_fd=0;
- char buffer[80];
char signature[16];
int rc, edd_version=0;
uint8_t mbr_type=0, signature_type=0;
@@ -514,11 +514,8 @@ make_disk_load_option(char *disk, uint8_t *buf, size_t size)
memset(signature, 0, sizeof(signature));
disk_fd = open(opts.disk, O_RDWR);
- if (disk_fd == -1) {
- sprintf(buffer, "Could not open disk %s", opts.disk);
- perror(buffer);
- return -1;
- }
+ if (disk_fd == -1)
+ err(5, "Could not open disk %s", opts.disk);
if (opts.edd_version) {
edd_version = get_edd_version();
@@ -539,12 +536,10 @@ make_disk_load_option(char *disk, uint8_t *buf, size_t size)
&part_start, &part_size, signature,
&mbr_type, &signature_type);
close(disk_fd);
- if (rc) {
- fprintf(stderr, "Error: no partition information on disk %s.\n"
- " Cowardly refusing to create a boot option.\n",
+ if (rc)
+ errx(5, "No partition information on disk %s.\n"
+ "Cowardly refusing to create a boot option.\n",
opts.disk);
- return -1;
- }
needed = make_harddrive_device_path(opts.part, part_start, part_size,
(uint8_t *)signature, mbr_type,
@@ -724,10 +719,13 @@ make_linux_load_option(uint8_t **data, size_t *data_size)
uint8_t *buf;
ssize_t needed;
off_t buf_offset = 0, desc_offset;
+ int rc;
load_option = calloc(1, sizeof (*load_option));
- if (load_option == NULL)
+ if (load_option == NULL) {
+ fprintf(stderr, "efibootmgr: %m\n");
return -1;
+ }
buf = (uint8_t *)load_option;
buf_offset = 0;
@@ -755,21 +753,33 @@ make_linux_load_option(uint8_t **data, size_t *data_size)
if (opts.iface) {
needed = make_net_load_option(opts.iface, NULL, 0);
if (needed < 0) {
+ fprintf(stderr, "efibootmgr: could not create load option: %m\n");
free(buf);
return needed;
}
buf = extend(load_option, load_option_size, needed);
- make_net_load_option(opts.iface, buf + buf_offset, needed);
+ rc = make_net_load_option(opts.iface, buf + buf_offset, needed);
buf_offset += needed;
+ if (rc < 0) {
+ fprintf(stderr, "efibootmgr: could not create load option: %m\n");
+ free(buf);
+ return rc;
+ }
} else {
needed = make_disk_load_option(opts.iface, NULL, 0);
if (needed < 0) {
+ fprintf(stderr, "efibootmgr: could not create load option: %m\n");
free(buf);
return needed;
}
buf = extend(load_option, load_option_size, needed);
- make_disk_load_option(opts.iface, buf + buf_offset, needed);
+ rc = make_disk_load_option(opts.iface, buf + buf_offset, needed);
buf_offset += needed;
+ if (rc < 0) {
+ fprintf(stderr, "efibootmgr: could not create load option: %m\n");
+ free(buf);
+ return rc;
+ }
}
load_option->file_path_list_length = buf_offset - desc_offset;
@@ -792,25 +802,25 @@ append_extra_args_ascii(uint8_t **data, size_t *data_size)
int i;
unsigned long usedchars=0;
- if (!data || *data)
+ if (!data || *data) {
+ errno = EINVAL;
return -1;
+ }
for (i=opts.optind; i < opts.argc; i++) {
- int l = strlen(opts.argv[i]) + 1;
+ int l = strlen(opts.argv[i]);
int space = (i < opts.argc - 1) ? 1: 0;
- uint8_t *tmp = realloc(new_data, (usedchars + l + space));
+ uint8_t *tmp = realloc(new_data, (usedchars + l + space + 1));
if (tmp == NULL)
return -1;
new_data = tmp;
p = (char *)new_data + usedchars;
strcpy(p, opts.argv[i]);
usedchars += l;
- p += l;
/* Put a space between args */
if (space)
- p[usedchars++] = ' ';
- else
- p[usedchars] = '\0';
+ new_data[usedchars++] = ' ';
+ new_data[usedchars] = '\0';
}
if (!new_data)
@@ -829,8 +839,10 @@ append_extra_args_unicode(uint8_t **data, size_t *data_size)
int i;
unsigned long usedchars=0;
- if (!data || *data)
+ if (!data || *data) {
+ errno = EINVAL;
return -1;
+ }
for (i = opts.optind; i < opts.argc; i++) {
int l = strlen(opts.argv[i]) + 1;
@@ -871,37 +883,31 @@ append_extra_args_file(uint8_t **data, size_t *data_size)
size_t maxchars = 0;
char *buffer;
- if (!data) {
- fprintf(stderr, "internal error\n");
- exit(1);
+ if (!data || *data) {
+ errno = EINVAL;
+ return -1;
}
if (file && strncmp(file, "-", 1))
fd = open(file, O_RDONLY);
- if (fd == -1) {
- perror("Failed to open extra arguments file");
- exit(1);
- }
+ if (fd < 0)
+ return -1;
buffer = malloc(maxchars);
do {
if (maxchars - appended == 0) {
maxchars += 1024;
char *tmp = realloc(buffer, maxchars);
- if (tmp == NULL) {
- perror("Error reading extra arguments file");
- exit(1);
- }
+ if (tmp == NULL)
+ return -1;
buffer = tmp;
}
num_read = read(fd, buffer + appended, maxchars - appended);
- if (num_read < 0) {
- perror("Error reading extra arguments file");
- exit(1);
- } else if (num_read > 0) {
+ if (num_read < 0)
+ return -1;
+ else if (num_read > 0)
appended += num_read;
- }
} while (num_read > 0);
if (fd != STDIN_FILENO)
@@ -935,14 +941,18 @@ append_extra_args(uint8_t **data, size_t *data_size)
if (opts.extra_opts_file) {
ret = append_extra_args_file(&new_data, &new_data_size);
- if (ret < 0)
+ if (ret < 0) {
+ fprintf(stderr, "efibootmgr: append_extra_args: %m\n");
return -1;
+ }
}
if (new_data_size) {
ret = add_new_data(data, data_size, new_data, new_data_size);
free(new_data);
- if (ret < 0)
+ if (ret < 0) {
+ fprintf(stderr, "efibootmgr: append_extra_args: %m\n");
return -1;
+ }
new_data = NULL;
new_data_size = 0;
}
@@ -952,6 +962,7 @@ append_extra_args(uint8_t **data, size_t *data_size)
else
ret = append_extra_args_ascii(&new_data, &new_data_size);
if (ret < 0) {
+ fprintf(stderr, "efibootmgr: append_extra_args: %m\n");
if (new_data) /* this can't happen, but covscan believes */
free(new_data);
return -1;
@@ -960,8 +971,10 @@ append_extra_args(uint8_t **data, size_t *data_size)
ret = add_new_data(data, data_size, new_data, new_data_size);
free(new_data);
new_data = NULL;
- if (ret < 0)
+ if (ret < 0) {
+ fprintf(stderr, "efibootmgr: append_extra_args: %m\n");
return -1;
+ }
new_data_size = 0;
}
diff --git a/src/man/man8/efibootmgr.8 b/src/man/man8/efibootmgr.8
index 96071d7..423bc16 100644
--- a/src/man/man8/efibootmgr.8
+++ b/src/man/man8/efibootmgr.8
@@ -93,9 +93,6 @@ Partition number containing the bootloader (defaults to 1)
\fB-q | --quiet\fR
Quiet mode - supresses output.
.TP
-\fB--test \fIfilename\fB\fR
-Don't write to NVRAM, write to \fIfilename\fR\&.
-.TP
\fB-t | --timeout \fIseconds\fB\fR
Boot Manager timeout, in \fIseconds\fR\&.
.TP
--
2.7.4