Dear 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
Good point.
+{
- int i = 0;
- for (; *s; s++)
if (*s == ';')i++;- return ++i;
+}
In this function I count how many times the ';' separator appears. I didn't found proper function at ./lib/string.c.
looks kind of like: return (strrchr(s, ';') - s) + 1;
The above code returns position of the last occurrence of the ';' separator.
+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.
You are right, removed.
+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()
OK,
+static int dfu_fill_entity(struct dfu_entity *dfu, char* s, int alt,
"char *s", not "char* s"
OK,
+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); }
I'd prefer to leave it as it is (since IMHO is more readable) with the addition of following code:
for (i = 0; i < dfu_alt_num; i++) { dfu = calloc(sizeof(struct dfu_entity), 1); if (!dfu) { dfu_free_entities(); return -1; } }
--- /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"
OK
-mike