[PATCH] add ubiupdatevol applet

[Baruch Siach]

Hi Reuben,

On Fri, Apr 01, 2011 at 02:37:20PM +1300, Reuben Dowle wrote:
> This patch adds the ubiupdatevol applet (from mtd-utils) to busybox.
> 
> This version of ubiupdatevol makes some assumptions about the naming of ubi device nodes, to avoid the complexity of scanning and probing devices nodes as libubi does in the full version of ubiupdatevol.
> 
> Signed-off-by: Reuben Dowle <reuben.dowle at navico.com>
> ---

[snip]

> @@ -102,12 +112,20 @@
>  //usage:     "\n	-n VOLID	Volume ID"
>  //usage:
>  //usage:#define ubirsvol_trivial_usage
> -//usage:       "UBI_DEVICE -N NAME -s SIZE"
> +//usage:       "UBI_DEVICE -n VOLID -s SIZE"

This is an unrelated bug fix. It should go in a separate patch.

>  //usage:#define ubirsvol_full_usage "\n\n"
>  //usage:       "Resize UBI Volume\n"
>  //usage:     "\nOptions:"
> -//usage:     "\n	-N NAME		Volume name"
> +//usage:     "\n	-n VOLID	Volume ID to resize"

Ditto.

>  //usage:     "\n	-s SIZE		Size in bytes"
> +//usage:
> +//usage:#define ubiupdatevol_trivial_usage
> +//usage:       "<UBI device node> UBI_VOL_DEV IMG_FILE"
> +//usage:#define ubiupdatevol_full_usage "\n\n"
> +//usage:       "Update UBI Volume\n"
> +//usage:     "\nOptions:"
> +//usage:     "\n	-t		truncate UBI volume"
> +//usage:     "\n	-s SIZE		bytes in input (if not reading from file)"
>  
>  int ubi_tools_main(int argc, char **argv) MAIN_EXTERNALLY_VISIBLE;
>  int ubi_tools_main(int argc UNUSED_PARAM, char **argv)

[snip]

> +		else
> +		{
> +			// Make assumption that device name is in normal format. Removes need for scanning sysfs tree as full libubi does
> +			if (sscanf(ubi_ctrl, "/dev/ubi%d_%d", &ubinum, &volnum) != 2)
> +				bb_error_msg_and_die("%s volume node not in correct format", "UBI");
> +
> +			snprintf(buf, sizeof(buf), "/sys/class/ubi/ubi%d_%d/usable_eb_size", ubinum, volnum);
> +			sysfs_fd = xopen(buf, O_RDONLY);
> +			if (!sysfs_fd)
> +				bb_error_msg_and_die("%s sysfs not accessible", "UBI");

Redundant. xopen() never returns on error. And besides, 0 is a valid file 
descriptor.

> +			if (full_read(sysfs_fd, buf, sizeof(buf)) <= 0)
> +				bb_error_msg_and_die("%s sysfs not accessible", "UBI");

A "not accessible" error message seems misleading when full_read() returns 0.

> +			if (ENABLE_FEATURE_CLEAN_UP)
> +				close(sysfs_fd);
> +			if (sscanf(buf, "%d", &leb_size) != 1)
> +				bb_error_msg_and_die("%s sysfs not accessible", "UBI");

The "not accessible" error message looks inappropriate here.

> +			input_data = (char *)xmalloc(leb_size);
> +			if (opts & OPTION_s)
> +				input_fd = 0;
> +			else
> +			{
> +				if (stat(argv[optind+1], &st))
> +					bb_error_msg_and_die("%s volume input file missing", "UBI");
> +				size_bytes = st.st_size;
> +				input_fd = xopen(argv[optind+1], O_RDONLY);
> +				if (!input_fd)
> +					bb_error_msg_and_die("%s volume input file missing", "UBI");

Redundant. See above.

> +			}
> +
> +			bytes = size_bytes;
> +			xioctl(fd, UBI_IOCVOLUP, &bytes);
> +
> +			while ((len = full_read(input_fd, input_data, leb_size)) > 0)
> +			{
> +				if (full_write(fd, input_data, leb_size) != leb_size)
> +					bb_error_msg_and_die("%s volume update failed", "UBI");
> +			}
> +			if (ENABLE_FEATURE_CLEAN_UP)
> +				close(input_fd);
> +			if (len < 0)
> +				bb_error_msg_and_die("%s volume update failed", "UBI");

I'd put this before the close() above. This should make it more apparent that 
this error handling relates to the full_read() above.

> +		}
>  	}
>  
>  	if (ENABLE_FEATURE_CLEAN_UP)
> 
> 
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.2.679.5364, http://www.tkos.co.il -