[Skiboot] [PATCH 4/7] libflash/file: Add a file access backend to for the blocklevel interface.

[Cyril Bur]

On Fri, 12 Jun 2015 12:39:01 +1000
Alistair Popple <alistair at popple.id.au> wrote:

> Cyril,
> 
> Comments below.
> 
> Regards,
> 
> Alistair
> 
> On Fri, 5 Jun 2015 14:11:28 Cyril Bur wrote:
> 
> <snip>
> 
> > +
> > +static int file_write(struct blocklevel_device *bl, uint32_t dst,
> > const void *src,
> > +		uint32_t len)
> > +{
> > +	struct file_data *file_data = container_of(bl, struct
> > file_data, bl);
> > +	int count = 0;
> > +	int rc;
> > +
> > +	rc = lseek(file_data->fd, dst, SEEK_SET);
> > +	/* errno should remain set */
> > +	if (rc == -1)
> > +		return FLASH_ERR_PARM_ERROR;
> > +
> > +	while (count < len) {
> > +		rc = write(file_data->fd, src, len);
> > +		/* errno should remain set */
> > +		if (rc == -1)
> > +			return FLASH_ERR_VERIFY_FAILURE;
> > +		count += rc;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int file_erase(struct blocklevel_device *bl, uint32_t dst,
> > uint32_t len)
> 
> Probably worth a comment here on what it actually means to "erase" a
> file.
> 

yeah added.

> > +{
> > +	unsigned long long int d = ULLONG_MAX;
> > +	int i = 0;
> > +	int rc = 0;
> > +
> > +	while (len - i > 0) {
> > +		rc = file_write(bl, dst + i, &d, len - i >
> > sizeof(d) ? sizeof(d) : len - i);
> > +		if (rc)
> > +			return rc;
> > +		i += sizeof(d);
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int mtd_erase(struct blocklevel_device *bl, uint32_t dst,
> > uint32_t len) +{
> > +	struct file_data *file_data = container_of(bl, struct
> > file_data, bl);
> > +	struct erase_info_user erase_info = {
> > +		.start = dst,
> > +		.length = len
> > +	};
> > +
> > +	return ioctl(file_data->fd, MEMERASE, &erase_info) == -1 ?
> > -1 : 0; +}
> > +
> > +static int get_info_name(struct file_data *file_data, char **name)
> > +{
> > +	char *path, *lpath;
> > +	int len;
> > +	struct stat st;
> > +
> > +	path = strdup("/proc/self/fd/");
> > +	if (!path)
> > +		return FLASH_ERR_MALLOC_FAILED;
> > +
> > +	/* strlen() + 1 to count the \0 */
> > +	len = snprintf(path, strlen(path) + 1, "%s%d", path,
> > file_data->fd);
> > +	while (len > 0 && len > strlen(path)) {
> > +		len++; /* count the /0 */
> > +		path = realloc(path, len);
> > +		if (!path)
> > +			return FLASH_ERR_MALLOC_FAILED;
> > +		len = snprintf(path, len, "%s%d", path,
> > file_data->fd);
> > +	}
> 
> What am I missing here? As this won't be used in skiboot couldn't you
> just do asprintf(path, len, "/proc/self/fd/%d", path, file_data->fd)?
> 

Well if I'd thought of that ;), yeah looks much nicer, also I could
have not leaked path... but you say anything so it didn't happen :).

> > +
> > +	if (len <= 0) {
> > +		free(path);
> > +		return FLASH_ERR_VERIFY_FAILURE;
> > +	}
> > +
> > +	if (lstat(path, &st)) {
> > +		free(path);
> > +		return FLASH_ERR_PARM_ERROR;
> > +	}
> > +
> > +	lpath = malloc(st.st_size + 1);
> > +	if (!lpath) {
> > +		free(path);
> > +		return FLASH_ERR_MALLOC_FAILED;
> > +	}
> > +
> > +	len = readlink(path, lpath, st.st_size +1);
> > +	if (len == -1) {
> > +		free(path);
> > +		free(lpath);
> > +		return FLASH_ERR_PARM_ERROR;
> > +	}
> > +	lpath[len] = '\0';
> > +
> > +	*name = lpath;
> > +
> > +	return 0;
> > +}
> > +
> > +
> > +static int mtd_get_info(struct blocklevel_device *bl, const char
> > **name, uint32_t *total_size,
> > +		uint32_t *erase_granule)
> > +{
> > +	struct file_data *file_data = container_of(bl, struct
> > file_data, bl);
> > +	struct mtd_info_user mtd_info;
> > +	int rc;
> > +
> > +	if (ioctl(file_data->fd, MEMGETINFO, &mtd_info) == -1)
> > +			return FLASH_ERR_BAD_READ;
> > +
> > +	if (total_size)
> > +		*total_size = mtd_info.size;
> > +
> > +	if (erase_granule)
> > +		*erase_granule = mtd_info.erasesize;
> > +
> > +	if (name) {
> > +		rc = get_info_name(file_data, &(file_data->name));
> > +		if (rc)
> > +			return rc;
> > +		*name = file_data->name;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +static int file_get_info(struct blocklevel_device *bl, const char
> > **name, uint32_t *total_size,
> > +		uint32_t *erase_granule)
> > +{
> > +	struct file_data *file_data = container_of(bl, struct
> > file_data, bl);
> > +	struct stat st;
> > +	int rc;
> > +
> > +	if (fstat(file_data->fd, &st))
> > +		return FLASH_ERR_PARM_ERROR;
> > +
> > +	if (total_size)
> > +		*total_size = st.st_size;
> > +
> > +	if (erase_granule)
> > +		*erase_granule = 1;
> > +
> > +	if (name) {
> > +		rc = get_info_name(file_data, &(file_data->name));
> > +		if (rc)
> > +			return rc;
> > +		*name = file_data->name;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> > +int file_init(int fd, struct blocklevel_device **bl)
> > +{
> > +	struct file_data *file_data;
> > +	struct stat sbuf;
> > +
> > +	if (!bl)
> > +		return FLASH_ERR_PARM_ERROR;
> > +
> > +	*bl = NULL;
> > +
> > +	file_data = malloc(sizeof(struct file_data));
> > +	if (!file_data)
> > +		return FLASH_ERR_MALLOC_FAILED;
> > +	memset(file_data, 0, sizeof(*file_data));
> > +
> > +	file_data->fd = fd;
> > +	file_data->bl.read = &file_read;
> > +	file_data->bl.write = &file_write;
> > +	file_data->bl.erase = &file_erase;
> > +	file_data->bl.get_info = &file_get_info;
> > +	file_data->bl.erase_mask = 0;
> > +	/*
> > +	 * Unfortunately not all file descriptors are created
> > equal...
> > +	 * Here we check to see if the file descriptor is to an
> > MTD device, in
> > +	 * which case we have to erase and get the size of it
> > differently.
> > +	 */
> > +	if (fstat(file_data->fd, &sbuf) == -1)
> > +		goto out;
> > +
> > +	/* Won't be able to handle other than MTD devices for now
> > */
> > +	if (S_ISCHR(sbuf.st_mode)) {
> > +		file_data->bl.erase = &mtd_erase;
> > +		file_data->bl.get_info = &mtd_get_info;
> > +		file_data->bl.flags = WRITE_NEED_ERASE;
> > +		mtd_get_info(&file_data->bl, NULL, NULL,
> > &(file_data->bl.erase_mask));
> > +		file_data->bl.erase_mask--;
> > +	} else if (!S_ISREG(sbuf.st_mode)) {
> > +		/* If not a char device or a regular file
> > something went wrong */
> > +		goto out;
> > +	}
> > +
> > +	*bl = &(file_data->bl);
> > +	return 0;
> > +out:
> > +	free(file_data);
> > +	return FLASH_ERR_PARM_ERROR;
> > +}
> > +
> > +int file_init_path(const char *path, int *r_fd, struct
> > blocklevel_device **bl) +{
> > +	int fd, rc;
> > +
> > +	if (!path || !bl)
> > +		return FLASH_ERR_PARM_ERROR;
> > +
> > +	fd = open(path, O_RDWR);
> > +	if (fd == -1)
> > +		return FLASH_ERR_PARM_ERROR;
> > +
> > +	rc = file_init(fd, bl);
> > +	if (rc)
> > +		close(fd);
> > +
> > +	if (r_fd)
> > +		*r_fd = fd;
> > +
> > +	return rc;
> > +}
> > +
> > +void file_exit(struct blocklevel_device *bl)
> > +{
> > +	struct file_data *file_data;
> > +	if (bl)
> > +		file_data = container_of(bl, struct file_data, bl);
> > +		free(file_data->name);
> > +		free(file_data);
> > +}
> > +
> > +void file_exit_close(struct blocklevel_device *bl)
> > +{
> > +	struct file_data *file_data;
> > +	if (bl) {
> > +		file_data = container_of(bl, struct file_data, bl);
> > +		close(file_data->fd);
> > +		file_exit(bl);
> > +	}
> > +}
> > diff --git a/libflash/file.h b/libflash/file.h
> > new file mode 100644
> > index 0000000..1ad9129
> > --- /dev/null
> > +++ b/libflash/file.h
> > @@ -0,0 +1,43 @@
> > +/* Copyright 2013-2015 IBM Corp.
> > + *
> > + * Licensed under the Apache License, Version 2.0 (the "License");
> > + * you may not use this file except in compliance with the License.
> > + * You may obtain a copy of the License at
> > + *
> > + * 	http://www.apache.org/licenses/LICENSE-2.0
> > + *
> > + * Unless required by applicable law or agreed to in writing,
> > software
> > + * distributed under the License is distributed on an "AS IS"
> > BASIS,
> > + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or
> > + * implied.
> > + * See the License for the specific language governing permissions
> > and
> > + * limitations under the License.
> > + */
> > +
> > +#ifndef __LIBFLASH_FILE_H
> > +#define __LIBFLASH_FILE_H
> > +
> > +/*
> > + * Blockevel functions created leave errno set on errors, as these
> > calls
> > + * often boil down to standard read() and write() calls,
> > inspecting errno
> > + * may prove useful
> > + */
> > +
> > +int file_init(int fd, struct blocklevel_device **bl);
> > +void file_exit(struct blocklevel_device *bl);
> > +
> > +/*
> > + * file_init_path() is a convenience wrapper which will open the
> > path and call
> > + * file_init(). The call to open happens with O_RDWR and no
> > additional flags
> > + * Because file_exit() doesn't close the file descriptor,
> > file_init_path()
> > + * makes it available.
> > + */
> > +int file_init_path(const char *path, int *fd, struct
> > blocklevel_device **bl); +
> > +/*
> > + * file_exit_close is a convenience wrapper which will close the
> > open
> > + * file descriptor and call file_exit().
> > + */
> > +void file_exit_close(struct blocklevel_device *bl);
> > +
> > +#endif /* __LIBFLASH_FILE_H */
> > 
> 
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot