[patch] Add flash_eraseall from mtd-utils

Sebastian Andrzej Siewior bigeasy at linutronix.de
Mon Feb 16 08:49:41 UTC 2009


* Denys Vlasenko | 2009-02-15 15:45:56 [+0100]:

>On Friday 13 February 2009 20:29, Sebastian Andrzej Siewior wrote:
>> +static struct jffs2_unknown_node cleanmarker;
>> +int target_endian = __BYTE_ORDER;
>> +static uint32_t *crc32_table;
>
>It's best to eliminate global variables.
I could see if I can get rid of the two static ones (which are not
globally visible anyway) but the target_endian is defined by a header
file.

>> +int flash_eraseall_main(int argc, char **argv);
>> +int flash_eraseall_main(int argc, char **argv)
>> +{
>> +	int quiet;
>> +	int jffs2;
>> +	mtd_info_t meminfo;
>> +	int fd, clmpos = 0, clmlen = 8;
>> +	erase_info_t erase;
>> +	struct stat st;
>> +	int isNAND, bbtest = 1;
>> +	unsigned int flags;
>> +	char *mtd_name;
>> +
>> +	if (argc < 2) {
>> +		bb_show_usage();
>> +	}
>
>This can be achieved with opt_complementary = "..."
I take a look on that.
>
>> +	flags = getopt32(argv, "jq");
>> +	jffs2 = flags & OPTION_J;
>> +	quiet = flags & OPTION_Q;
>
>What's the point? You can just use (flags & OPTION_J) instead of jffs2
>in the code below?
I could, will change it that way.

>
>> +	mtd_name = *(argv + optind);
>> +
>> +	xstat(mtd_name, &st);
>
>SEGV. mtd_name can be NULL here.
oh yes.

>
>> +	if (!S_ISCHR(st.st_mode)) {
>> +		bb_error_msg_and_die("%s: not a char device", mtd_name);
>> +	}
>> +
>> +	fd = xopen(mtd_name, O_RDWR);
>> +
>> +	xioctl(fd, MEMGETINFO, &meminfo);
>> +
>> +	erase.length = meminfo.erasesize;
>> +	isNAND = meminfo.type == MTD_NANDFLASH ? 1 : 0;
>> +
>> +	if (jffs2) {
>> +		crc32_table = crc32_filltable(NULL, 0);
>> +
>> +		cleanmarker.magic = cpu_to_je16 (JFFS2_MAGIC_BITMASK);
>> +		cleanmarker.nodetype = cpu_to_je16 (JFFS2_NODETYPE_CLEANMARKER);
>> +		if (!isNAND)
>> +			cleanmarker.totlen = cpu_to_je32 (sizeof (struct jffs2_unknown_node));
>> +		else {
>> +			struct nand_oobinfo oobinfo;
>> +
>> +			xioctl(fd, MEMGETOOBSEL, &oobinfo);
>> +
>> +			/* Check for autoplacement */
>> +			if (oobinfo.useecc == MTD_NANDECC_AUTOPLACE) {
>> +				/* Get the position of the free bytes */
>> +				if (!oobinfo.oobfree[0][1]) {
>> +					fprintf (stderr, " Eeep. Autoplacement selected and no empty space in oob\n");
>
>We have bb_error_msg("...") for that.
>" Eeep. " is bloat and should be dropped.
Okay.

>
>--
>vda
thx for the review.

Sebastian


More information about the busybox mailing list