[U-Boot] [PATCH v3 3/7] dfu: DFU backend implementation

[Mike Frysinger]

On Tuesday 31 July 2012 02:36:59 Lukasz Majewski wrote:
> --- /dev/null
> +++ b/drivers/dfu/dfu.c
>
> +static int dfu_find_alt_num(char *s)

const char *s

> +{
> +	int i = 0;
> +
> +	for (; *s; s++)
> +		if (*s == ';')
> +			i++;
> +
> +	return ++i;
> +}

looks kind of like:
	return (strrchr(s, ';') - s) + 1;

> +int dfu_write(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
> +{
> +	static unsigned char *i_buf;
> +	static int i_blk_seq_num;
> +	long w_size = 0;
> +	int ret = 0;
> +
> +	if (blk_seq_num == 0) {
> +		memset(dfu_buf, '\0', sizeof(dfu_buf));
> ...
> +	memcpy(i_buf, buf, size);
> +	i_buf += size;

why bother clearing it ?  since right below we memcpy() in the data we care 
about from buf, i'd skip the memset() completely.

> +int dfu_read(struct dfu_entity *dfu, void *buf, int size, int blk_seq_num)
> +{
> +	static unsigned char *i_buf;
> +	static int i_blk_seq_num;
> +	static long r_size;
> +	static u32 crc;
> +	int ret = 0;
> +
> +	if (blk_seq_num == 0) {
> +		i_buf = dfu_buf;
> +		memset(dfu_buf, '\0', sizeof(dfu_buf));
> +		ret = dfu->read_medium(dfu, i_buf, &r_size);
> +		debug("%s: %s %ld [B]\n", __func__, dfu->name, r_size);
> +		i_blk_seq_num = 0;
> +		/* Integrity check (if needed) */
> +		crc = crc32(0, dfu_buf, r_size);
> +	}

same here -- punt the memset()

> +static int dfu_fill_entity(struct dfu_entity *dfu, char* s, int alt,

"char *s", not "char* s"

> +int dfu_config_entities(char *env, char *interface, int num)
> +{
> +	struct dfu_entity *dfu;
> +	int i, ret;
> +	char *s;
> +
> +	dfu_alt_num = dfu_find_alt_num(env);
> +	debug("%s: dfu_alt_num=%d\n", __func__, dfu_alt_num);
> +
> +	for (i = 0; i < dfu_alt_num; i++) {
> +		dfu = calloc(sizeof(struct dfu_entity), 1);

seems like you can do this in a single call outside of the for loop:
	dfu = calloc(sizeof(*dfu), dfu_alt_num);
	if (!dfu)
		return -1;
	for (i = 0; i < dfu_alt_num; i++) {
		s = strsep(&env, ";");
		ret = dfu_fill_entity(&dfu[i], s, i, interface, num);
		if (ret)
			return -1;
		list_add_tail(&dfu[i].list, &dfu_list);
	}

> --- /dev/null
> +++ b/include/dfu.h
>
> +char *dfu_extract_token(char** e, int *n);
> +extern int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s);
> +static inline int dfu_fill_entity_mmc(struct dfu_entity *dfu, char* s)

"char *s", not "char* s"
-mike
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 836 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20120801/c7c6b43f/attachment.pgp>