[PATCH v4 2/2] lib: Support to deal with petitboot's configuration on efi-based platforms

[Geoff Levand]

Hi Ge,

On 05/25/2018 08:53 PM, Ge Song wrote:
> +++ b/lib/efi/efivar.h> +#ifndef EFIVAR_H
> +#define EFIVAR_H
> 
> +#ifndef EFIVARFS_MAGIC
> +#define EFIVARFS_MAGIC 0xde5e81e4

Do we want to be supporting libc so old it does not have EFIVARFS_MAGIC?
Maybe emit an error here:

#error Upgrade libc to a newer version with EFIVARFS_MAGIC

> +#endif

> diff --git a/lib/efi/efivar.c b/lib/efi/efivar.c
> new file mode 100644
> index 000000000000..8e6fce820bfb
> --- /dev/null
> +++ b/lib/efi/efivar.c

The only use of ctx in the efivar.c functions is to attach temp variables
like path and buf.  We shouldn't be adding temp variables to contexts.
I think it better to not have ctx, then delete any temp variables on
function exit.

> +#include "talloc/talloc.h"
> +
> +const char *efivarfs_path = NULL;

efivarfs_path is a global, so this initialization is
unneeded.  It will be initialized to NULL.

> +
> +inline void set_efivarfs_path(const char *path)
> +int efi_get_variable(void *ctx, const char *guidstr, const char *name,
> +		uint8_t **data, size_t *data_size, uint32_t *attributes)

So use int efi_get_variable(const char *guidstr, ...)

> +{
> +	int fd, errno_value;
> +	int rc = -1;
> +	void *p, *buf;
> +	size_t bufsize = 4096;
> +	size_t filesize = 0;
> +	ssize_t sz;
> +	const char *dir;
> +	char *path;
> +
> +	dir = get_efivarfs_path();
> +	if (!dir)
> +		return EFAULT;
> +
> +	path = talloc_asprintf(ctx, "%s%s-%s", dir, name, guidstr);

Use talloc_asprintf(NULL, ...)

> +	if (!path)
> +		return ENOMEM;
> +
> +	fd = open(path, O_RDONLY|O_NONBLOCK);
> +	if (fd < 0)
> +		goto err;
> +
> +	buf = talloc_size(ctx, bufsize);

and talloc_size(path, ...)

> +	if (!buf)
> +		goto err;
> +
> +	do {
> +		p = buf + filesize;
> +		sz = read(fd, p, bufsize);
> +		if (sz < 0 && errno == EAGAIN) {
> +			continue;
> +		} else if (sz == 0) {
> +			break;
> +		}
> +		filesize += sz;
> +	} while (1);
> +
> +	*attributes = *(uint32_t *)buf;
> +	*data = (uint8_t *)(buf + sizeof(uint32_t));
> +	*data_size = strlen(buf + sizeof(uint32_t));
> +	rc = 0;
> +
> +err:

then talloc_free(path)

> +	errno_value = errno;
> +	if (fd > 0)
> +		close(fd);
> +
> +	errno = errno_value;
> +	return rc;
> +}