On Friday 22 May 2009 11:08, Thierry Reding wrote:
> Signed-off-by: Thierry Reding <[email protected]>
> ---
> include/applets.h | 2 +
> include/usage.h | 11 ++++++
> miscutils/Config.in | 20 ++++++++++++
> miscutils/Kbuild | 1 +
> miscutils/flash_lock_unlock.c | 70
> +++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 104 insertions(+), 0 deletions(-)
> create mode 100644 miscutils/flash_lock_unlock.c
>
> diff --git a/include/applets.h b/include/applets.h
> index 7838757..8ce81a2 100644
> --- a/include/applets.h
> +++ b/include/applets.h
> @@ -155,6 +155,8 @@ IF_FDISK(APPLET(fdisk, _BB_DIR_SBIN, _BB_SUID_NEVER))
> IF_FEATURE_GREP_FGREP_ALIAS(APPLET_ODDNAME(fgrep, grep, _BB_DIR_BIN,
> _BB_SUID_NEVER, fgrep))
> IF_FIND(APPLET_NOEXEC(find, find, _BB_DIR_USR_BIN, _BB_SUID_NEVER, find))
> IF_FINDFS(APPLET(findfs, _BB_DIR_SBIN, _BB_SUID_MAYBE))
> +IF_FLASH_LOCK(APPLET_ODDNAME(flash_lock, flash_lock_unlock,
> _BB_DIR_USR_SBIN, _BB_SUID_NEVER, flash_lock))
> +IF_FLASH_UNLOCK(APPLET_ODDNAME(flash_unlock, flash_lock_unlock,
> _BB_DIR_USR_SBIN, _BB_SUID_NEVER, flash_unlock))
> //IF_FLASH_ERASEALL(APPLET_ODDNAME(flash_eraseall, flash_eraseall,
> _BB_DIR_USR_SBIN, _BB_SUID_NEVER, flash_eraseall))
> IF_FLASH_ERASEALL(APPLET(flash_eraseall, _BB_DIR_USR_SBIN, _BB_SUID_NEVER))
Wrong order. The comment in this file says entries must be
in ascending order.
> +config FLASH_LOCK_UNLOCK
> + bool
> + default n
> +
> +config FLASH_LOCK
> + bool "flash_lock"
> + select FLASH_LOCK_UNLOCK
> + default n
> + help
> + The flash_lock binary from mtd-utils as of git head 5ec0c10d0. This
> + utility locks part or all of the flash device.
> +
> +config FLASH_UNLOCK
> + bool "flash_unlock"
> + select FLASH_LOCK_UNLOCK
> + default n
> + help
> + The flash_unlock binary from mtd-utils as of git head 5ec0c10d0. This
> + utility unlocks part or all of the flash device.
> +
Isn't it simpler to have two options instead of three?
> + if (strcmp(argv[0], "flash_unlock") == 0)
> + do_lock = 0;
argv[0] can be "/bin/flash_unlock". Need to look at applet_name.
> + if (strncmp(argv[1], "/dev/mtd", 8) != 0) {
> + bb_error_msg_and_die("'%s' is not an MTD device. Must specify "
> + "an MTD devic: /dev/mtd?\n", argv[1]);
> + }
Not useful.
> + fd = xopen(argv[1], O_RDWR);
> +
> + if (xioctl(fd, MEMGETINFO, &info)) {
> + bb_error_msg_and_die("failed to get MTD device info from %s\n",
> + argv[1]);
> + }
xioctl does not return on error.
> + xioctl(fd, do_lock ? MEMLOCK : MEMUNLOCK, &lock)
xioctl is a macro. If you use it like that, do_lock ? MEMLOCK : MEMUNLOCK
will get converted to "do_lock ? MEMLOCK : MEMUNLOCK" string
in error message. Not good.
I fixed these problems and added it to git repo. Please test
and let me know if something doesn't work.
Thanks!
--
vda
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://lists.busybox.net/mailman/listinfo/busybox
