new applet: devmem

[Vladimir Dronnikov]

why not just dd? explain me, please


On 10/26/08, Bastian Blank <waldi at debian.org> wrote:
> On Sun, Oct 26, 2008 at 05:10:45AM -0400, Mike Frysinger wrote:
>> +#define devmem_full_usage \
>> +	"Read/Write from physical addresses" \
>> +	"\n\nUsage:  devmem { address } [ type [ data ] ]" \
>> +	"\n	address : memory address to act upon" \
>> +	"\n	type    : access operation type : [b]yte, [h]alfword, [w]ord" \
>> +	"\n	data    : data to be written"
>
> Where is the definition of halfword and word? I assume 16 and 32 bit.
> And what about 64 bit access?
>
>> +config DEVMEM
>> +	bool "devmem"
>> +	default y
>
> Why default y? This is no generic tool.
>
>> +#define DEVMEM_MAP_SIZE 4096UL
>
> I think this needs to match the pagesize, which is not fixed to 4096.
>
>> +int devmem_main(int argc, char **argv) {
>> +	void *map_base, *virt_addr;
>> +	unsigned long read_result, writeval;
>> +	off_t target;
>> +	int fd, access_type = 'w';
>
> Default value is not documented in the help.
>
>> +	if (argc < 2)
>> +		bb_show_usage();
>> +
>> +	target = bb_strtoul(argv[1], 0, 0);
>> +
>> +	if (argc > 2)
>> +		access_type = tolower(argv[2][0]);
>> +
>> +	fd = xopen("/dev/mem", O_RDWR | O_SYNC);
>> +
>> +	if ((map_base = mmap(0, DEVMEM_MAP_SIZE, PROT_READ | PROT_WRITE,
>> MAP_SHARED, fd, target & ~DEVMEM_MAP_MASK)) == MAP_FAILED)
>
> Where is overflow handled?
>
>> +	printf("Memory mapped at address %p.\n", map_base);
>> +
>> +	virt_addr = map_base + (target & DEVMEM_MAP_MASK);
>
> map_base is void *, aka no aritmetic allowed.
>
>> +	if (access_type == 'b')
>> +		read_result = *((unsigned char *) virt_addr);
>> +	else if (access_type == 'h')
>> +		read_result = *((unsigned short *) virt_addr);
>> +	else if (access_type == 'w')
>> +		read_result = *((unsigned long *) virt_addr);
>
> This definitions are odd. long is 64bit on several arches.
>
>> +	else {
>> +		fprintf(stderr, "Illegal data type '%c'\n", access_type);
>> +		exit(EXIT_FAILURE);
>
> This belongs to the argument check.
>
>> +	if (argc > 3) {
>> +		writeval = bb_strtoul(argv[3], 0, 0);
>
> Error checks?
>
>> +		if (access_type == 'b') {
>> +			*((unsigned char *) virt_addr) = writeval;
>> +			read_result = *((unsigned char *) virt_addr);
>> +		} else if (access_type == 'h') {
>> +			*((unsigned short *) virt_addr) = writeval;
>> +			read_result = *((unsigned short *) virt_addr);
>> +		} else if (access_type == 'w') {
>> +			*((unsigned long *) virt_addr) = writeval;
>> +			read_result = *((unsigned long *) virt_addr);
>> +		}
>> +		printf("Written 0x%X; readback 0x%X\n", writeval, read_result);
>> +	}
>> +
>> +	if (munmap(map_base, DEVMEM_MAP_SIZE) == -1)
>> +		bb_perror_msg_and_die("munmap");
>> +	close(fd);
>> +	fflush_stdout_and_exit(EXIT_SUCCESS);
>
> Why do you cleanup here but not on the error paths?
>
> Bastian
>
> --
> 	"That unit is a woman."
> 	"A mass of conflicting impulses."
> 		-- Spock and Nomad, "The Changeling", stardate 3541.9
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://busybox.net/cgi-bin/mailman/listinfo/busybox
>

-- 
Sent from Gmail for mobile | mobile.google.com