| Submitter | Antony Pavlov |
|---|---|
| Date | 2015-06-14 23:05:39 |
| Message ID | <1434323139-2009-1-git-send-email-antonynpavlov@gmail.com> |
| Download | mbox | patch |
| Permalink | /patch/4301/ |
| State | New |
| Headers | show |
Comments
On Mon, 15 Jun 2015 02:05:39 +0300 Antony Pavlov <antonynpavlov@gmail.com> wrote: Stefan! This patch make it possible to eliminate MASTERS_MAX limit. Could you please comment on this patch? > Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> > --- > cli_classic.c | 13 +++--- > flashrom.c | 1 - > list.h | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ > programmer.c | 27 ++++++------ > programmer.h | 7 ++-- > 5 files changed, 153 insertions(+), 25 deletions(-) > > diff --git a/cli_classic.c b/cli_classic.c > index a2c2014..06ae454 100644 > --- a/cli_classic.c > +++ b/cli_classic.c > @@ -96,7 +96,7 @@ int main(int argc, char *argv[]) > struct flashctx flashes[8] = {{0}}; > struct flashctx *fill_flash; > const char *name; > - int namelen, opt, i, j; > + int namelen, opt, i; > int startchip = -1, chipcount = 0, option_index = 0, force = 0; > #if CONFIG_PRINT_WIKI == 1 > int list_supported_wiki = 0; > @@ -134,6 +134,7 @@ int main(int argc, char *argv[]) > #endif /* !STANDALONE */ > char *tempstr = NULL; > char *pparam = NULL; > + struct registered_master *master; > > print_version(); > print_banner(); > @@ -425,10 +426,10 @@ int main(int argc, char *argv[]) > msg_pdbg("The following protocols are supported: %s.\n", tempstr); > free(tempstr); > > - for (j = 0; j < registered_master_count; j++) { > + list_for_each_entry(master, ®istered_masters, node) { > startchip = 0; > while (chipcount < ARRAY_SIZE(flashes)) { > - startchip = probe_flash(®istered_masters[j], startchip, &flashes[chipcount], 0); > + startchip = probe_flash(master, startchip, &flashes[chipcount], 0); > if (startchip == -1) > break; > chipcount++; > @@ -455,8 +456,7 @@ int main(int argc, char *argv[]) > int compatible_masters = 0; > msg_cinfo("Force read (-f -r -c) requested, pretending the chip is there:\n"); > /* This loop just counts compatible controllers. */ > - for (j = 0; j < registered_master_count; j++) { > - mst = ®istered_masters[j]; > + list_for_each_entry(mst, ®istered_masters, node) { > /* chip is still set from the chip_to_probe earlier in this function. */ > if (mst->buses_supported & chip->bustype) > compatible_masters++; > @@ -469,8 +469,7 @@ int main(int argc, char *argv[]) > if (compatible_masters > 1) > msg_cinfo("More than one compatible controller found for the requested flash " > "chip, using the first one.\n"); > - for (j = 0; j < registered_master_count; j++) { > - mst = ®istered_masters[j]; > + list_for_each_entry(mst, ®istered_masters, node) { > startchip = probe_flash(mst, 0, &flashes[0], 1); > if (startchip != -1) > break; > diff --git a/flashrom.c b/flashrom.c > index a389cb2..bd770c2 100644 > --- a/flashrom.c > +++ b/flashrom.c > @@ -494,7 +494,6 @@ int programmer_shutdown(void) > } > > programmer_param = NULL; > - registered_master_count = 0; > > return ret; > } > diff --git a/list.h b/list.h > new file mode 100644 > index 0000000..7567742 > --- /dev/null > +++ b/list.h > @@ -0,0 +1,130 @@ > +#ifndef LIST_H > +#define LIST_H > + > +/* > + * Copied from include/linux/... > + */ > + > +#undef offsetof > +#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER) > + > +/** > + * container_of - cast a member of a structure out to the containing structure > + * @ptr: the pointer to the member. > + * @type: the type of the container struct this is embedded in. > + * @member: the name of the member within the struct. > + * > + */ > +#define container_of(ptr, type, member) ({ \ > + const typeof( ((type *)0)->member ) *__mptr = (ptr); \ > + (type *)( (char *)__mptr - offsetof(type,member) );}) > + > + > +struct list_head { > + struct list_head *next, *prev; > +}; > + > +#define LIST_HEAD_INIT(name) { &(name), &(name) } > + > +#define LIST_HEAD(name) \ > + struct list_head name = LIST_HEAD_INIT(name) > + > +/** > + * list_entry - get the struct for this entry > + * @ptr: the &struct list_head pointer. > + * @type: the type of the struct this is embedded in. > + * @member: the name of the list_head within the struct. > + */ > +#define list_entry(ptr, type, member) \ > + container_of(ptr, type, member) > + > +/** > + * list_for_each_entry - iterate over list of given type > + * @pos: the type * to use as a loop cursor. > + * @head: the head for your list. > + * @member: the name of the list_head within the struct. > + */ > +#define list_for_each_entry(pos, head, member) \ > + for (pos = list_entry((head)->next, typeof(*pos), member); \ > + &pos->member != (head); \ > + pos = list_entry(pos->member.next, typeof(*pos), member)) > + > +/** > + * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry > + * @pos: the type * to use as a loop cursor. > + * @n: another type * to use as temporary storage > + * @head: the head for your list. > + * @member: the name of the list_head within the struct. > + */ > +#define list_for_each_entry_safe(pos, n, head, member) \ > + for (pos = list_entry((head)->next, typeof(*pos), member), \ > + n = list_entry(pos->member.next, typeof(*pos), member); \ > + &pos->member != (head); \ > + pos = n, n = list_entry(n->member.next, typeof(*n), member)) > + > +/** > + * list_empty - tests whether a list is empty > + * @head: the list to test. > + */ > +static inline int list_empty(const struct list_head *head) > +{ > + return head->next == head; > +} > + > +/* > + * Insert a new entry between two known consecutive entries. > + * > + * This is only for internal list manipulation where we know > + * the prev/next entries already! > + */ > +static inline void __list_add(struct list_head *_new, > + struct list_head *prev, > + struct list_head *next) > +{ > + next->prev = _new; > + _new->next = next; > + _new->prev = prev; > + prev->next = _new; > +} > + > +/** > + * list_add_tail - add a new entry > + * @new: new entry to be added > + * @head: list head to add it before > + * > + * Insert a new entry before the specified head. > + * This is useful for implementing queues. > + */ > +static inline void list_add_tail(struct list_head *_new, struct list_head *head) > +{ > + __list_add(_new, head->prev, head); > +} > + > +/* > + * Delete a list entry by making the prev/next entries > + * point to each other. > + * > + * This is only for internal list manipulation where we know > + * the prev/next entries already! > + */ > +static inline void __list_del(struct list_head *prev, struct list_head *next) > +{ > + next->prev = prev; > + prev->next = next; > +} > + > +#define LIST_POISON1 ((void *) 0x00100100) > +#define LIST_POISON2 ((void *) 0x00200200) > +/** > + * list_del - deletes entry from list. > + * @entry: the element to delete from the list. > + * Note: list_empty() on entry does not return true after this, the entry is > + * in an undefined state. > + */ > +static inline void list_del(struct list_head *entry) > +{ > + __list_del(entry->prev, entry->next); > + entry->next = (struct list_head*)LIST_POISON1; > + entry->prev = (struct list_head*)LIST_POISON2; > +} > +#endif > diff --git a/programmer.c b/programmer.c > index 1b27c3c..2a5df92 100644 > --- a/programmer.c > +++ b/programmer.c > @@ -18,6 +18,8 @@ > * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA > */ > > +#include <malloc.h> > +#include <string.h> > #include "flash.h" > #include "programmer.h" > > @@ -114,31 +116,28 @@ int register_par_master(const struct par_master *mst, > } > > /* The limit of 4 is totally arbitrary. */ > -#define MASTERS_MAX 4 > -struct registered_master registered_masters[MASTERS_MAX]; > -int registered_master_count = 0; > +LIST_HEAD(registered_masters); > > /* This function copies the struct registered_master parameter. */ > -int register_master(const struct registered_master *mst) > +int register_master(struct registered_master *mst) > { > - if (registered_master_count >= MASTERS_MAX) { > - msg_perr("Tried to register more than %i master " > - "interfaces.\n", MASTERS_MAX); > - return ERROR_FLASHROM_LIMIT; > - } > - registered_masters[registered_master_count] = *mst; > - registered_master_count++; > + struct registered_master *t; > + > + t = malloc(sizeof(struct registered_master)); > + memcpy(t, mst, sizeof(struct registered_master)); > + list_add_tail(&t->node, ®istered_masters); > > return 0; > } > > enum chipbustype get_buses_supported(void) > { > - int i; > + struct registered_master *mst; > enum chipbustype ret = BUS_NONE; > > - for (i = 0; i < registered_master_count; i++) > - ret |= registered_masters[i].buses_supported; > + list_for_each_entry(mst, ®istered_masters, node) { > + ret |= mst->buses_supported; > + } > > return ret; > } > diff --git a/programmer.h b/programmer.h > index 913522b..54d5634 100644 > --- a/programmer.h > +++ b/programmer.h > @@ -25,6 +25,7 @@ > #define __PROGRAMMER_H__ 1 > > #include "flash.h" /* for chipaddr and flashctx */ > +#include "list.h" > > enum programmer { > #if CONFIG_INTERNAL == 1 > @@ -691,6 +692,7 @@ struct par_master { > }; > int register_par_master(const struct par_master *mst, const enum chipbustype buses); > struct registered_master { > + struct list_head node; > enum chipbustype buses_supported; > union { > struct par_master par; > @@ -698,9 +700,8 @@ struct registered_master { > struct opaque_master opaque; > }; > }; > -extern struct registered_master registered_masters[]; > -extern int registered_master_count; > -int register_master(const struct registered_master *mst); > +extern struct list_head registered_masters; > +int register_master(struct registered_master *mst); > > /* serprog.c */ > #if CONFIG_SERPROG == 1 > -- > 2.1.4 >
On Wed, 17 Jun 2015 10:46:54 +0300 Antony Pavlov <antonynpavlov@gmail.com> wrote: > Stefan! > > This patch make it possible to eliminate MASTERS_MAX limit. > > Could you please comment on this patch? Hi, we need to integrate a lot of other patches before starting with such fundamental changes like adding generic containers/collection. If we would do it the other way round we would have to rewrite/fix a lot of *existing* patches.
Patch
diff --git a/cli_classic.c b/cli_classic.c index a2c2014..06ae454 100644 --- a/cli_classic.c +++ b/cli_classic.c @@ -96,7 +96,7 @@ int main(int argc, char *argv[]) struct flashctx flashes[8] = {{0}}; struct flashctx *fill_flash; const char *name; - int namelen, opt, i, j; + int namelen, opt, i; int startchip = -1, chipcount = 0, option_index = 0, force = 0; #if CONFIG_PRINT_WIKI == 1 int list_supported_wiki = 0; @@ -134,6 +134,7 @@ int main(int argc, char *argv[]) #endif /* !STANDALONE */ char *tempstr = NULL; char *pparam = NULL; + struct registered_master *master; print_version(); print_banner(); @@ -425,10 +426,10 @@ int main(int argc, char *argv[]) msg_pdbg("The following protocols are supported: %s.\n", tempstr); free(tempstr); - for (j = 0; j < registered_master_count; j++) { + list_for_each_entry(master, ®istered_masters, node) { startchip = 0; while (chipcount < ARRAY_SIZE(flashes)) { - startchip = probe_flash(®istered_masters[j], startchip, &flashes[chipcount], 0); + startchip = probe_flash(master, startchip, &flashes[chipcount], 0); if (startchip == -1) break; chipcount++; @@ -455,8 +456,7 @@ int main(int argc, char *argv[]) int compatible_masters = 0; msg_cinfo("Force read (-f -r -c) requested, pretending the chip is there:\n"); /* This loop just counts compatible controllers. */ - for (j = 0; j < registered_master_count; j++) { - mst = ®istered_masters[j]; + list_for_each_entry(mst, ®istered_masters, node) { /* chip is still set from the chip_to_probe earlier in this function. */ if (mst->buses_supported & chip->bustype) compatible_masters++; @@ -469,8 +469,7 @@ int main(int argc, char *argv[]) if (compatible_masters > 1) msg_cinfo("More than one compatible controller found for the requested flash " "chip, using the first one.\n"); - for (j = 0; j < registered_master_count; j++) { - mst = ®istered_masters[j]; + list_for_each_entry(mst, ®istered_masters, node) { startchip = probe_flash(mst, 0, &flashes[0], 1); if (startchip != -1) break; diff --git a/flashrom.c b/flashrom.c index a389cb2..bd770c2 100644 --- a/flashrom.c +++ b/flashrom.c @@ -494,7 +494,6 @@ int programmer_shutdown(void) } programmer_param = NULL; - registered_master_count = 0; return ret; } diff --git a/list.h b/list.h new file mode 100644 index 0000000..7567742 --- /dev/null +++ b/list.h @@ -0,0 +1,130 @@ +#ifndef LIST_H +#define LIST_H + +/* + * Copied from include/linux/... + */ + +#undef offsetof +#define offsetof(TYPE, MEMBER) ((size_t) &((TYPE *)0)->MEMBER) + +/** + * container_of - cast a member of a structure out to the containing structure + * @ptr: the pointer to the member. + * @type: the type of the container struct this is embedded in. + * @member: the name of the member within the struct. + * + */ +#define container_of(ptr, type, member) ({ \ + const typeof( ((type *)0)->member ) *__mptr = (ptr); \ + (type *)( (char *)__mptr - offsetof(type,member) );}) + + +struct list_head { + struct list_head *next, *prev; +}; + +#define LIST_HEAD_INIT(name) { &(name), &(name) } + +#define LIST_HEAD(name) \ + struct list_head name = LIST_HEAD_INIT(name) + +/** + * list_entry - get the struct for this entry + * @ptr: the &struct list_head pointer. + * @type: the type of the struct this is embedded in. + * @member: the name of the list_head within the struct. + */ +#define list_entry(ptr, type, member) \ + container_of(ptr, type, member) + +/** + * list_for_each_entry - iterate over list of given type + * @pos: the type * to use as a loop cursor. + * @head: the head for your list. + * @member: the name of the list_head within the struct. + */ +#define list_for_each_entry(pos, head, member) \ + for (pos = list_entry((head)->next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = list_entry(pos->member.next, typeof(*pos), member)) + +/** + * list_for_each_entry_safe - iterate over list of given type safe against removal of list entry + * @pos: the type * to use as a loop cursor. + * @n: another type * to use as temporary storage + * @head: the head for your list. + * @member: the name of the list_head within the struct. + */ +#define list_for_each_entry_safe(pos, n, head, member) \ + for (pos = list_entry((head)->next, typeof(*pos), member), \ + n = list_entry(pos->member.next, typeof(*pos), member); \ + &pos->member != (head); \ + pos = n, n = list_entry(n->member.next, typeof(*n), member)) + +/** + * list_empty - tests whether a list is empty + * @head: the list to test. + */ +static inline int list_empty(const struct list_head *head) +{ + return head->next == head; +} + +/* + * Insert a new entry between two known consecutive entries. + * + * This is only for internal list manipulation where we know + * the prev/next entries already! + */ +static inline void __list_add(struct list_head *_new, + struct list_head *prev, + struct list_head *next) +{ + next->prev = _new; + _new->next = next; + _new->prev = prev; + prev->next = _new; +} + +/** + * list_add_tail - add a new entry + * @new: new entry to be added + * @head: list head to add it before + * + * Insert a new entry before the specified head. + * This is useful for implementing queues. + */ +static inline void list_add_tail(struct list_head *_new, struct list_head *head) +{ + __list_add(_new, head->prev, head); +} + +/* + * Delete a list entry by making the prev/next entries + * point to each other. + * + * This is only for internal list manipulation where we know + * the prev/next entries already! + */ +static inline void __list_del(struct list_head *prev, struct list_head *next) +{ + next->prev = prev; + prev->next = next; +} + +#define LIST_POISON1 ((void *) 0x00100100) +#define LIST_POISON2 ((void *) 0x00200200) +/** + * list_del - deletes entry from list. + * @entry: the element to delete from the list. + * Note: list_empty() on entry does not return true after this, the entry is + * in an undefined state. + */ +static inline void list_del(struct list_head *entry) +{ + __list_del(entry->prev, entry->next); + entry->next = (struct list_head*)LIST_POISON1; + entry->prev = (struct list_head*)LIST_POISON2; +} +#endif diff --git a/programmer.c b/programmer.c index 1b27c3c..2a5df92 100644 --- a/programmer.c +++ b/programmer.c @@ -18,6 +18,8 @@ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ +#include <malloc.h> +#include <string.h> #include "flash.h" #include "programmer.h" @@ -114,31 +116,28 @@ int register_par_master(const struct par_master *mst, } /* The limit of 4 is totally arbitrary. */ -#define MASTERS_MAX 4 -struct registered_master registered_masters[MASTERS_MAX]; -int registered_master_count = 0; +LIST_HEAD(registered_masters); /* This function copies the struct registered_master parameter. */ -int register_master(const struct registered_master *mst) +int register_master(struct registered_master *mst) { - if (registered_master_count >= MASTERS_MAX) { - msg_perr("Tried to register more than %i master " - "interfaces.\n", MASTERS_MAX); - return ERROR_FLASHROM_LIMIT; - } - registered_masters[registered_master_count] = *mst; - registered_master_count++; + struct registered_master *t; + + t = malloc(sizeof(struct registered_master)); + memcpy(t, mst, sizeof(struct registered_master)); + list_add_tail(&t->node, ®istered_masters); return 0; } enum chipbustype get_buses_supported(void) { - int i; + struct registered_master *mst; enum chipbustype ret = BUS_NONE; - for (i = 0; i < registered_master_count; i++) - ret |= registered_masters[i].buses_supported; + list_for_each_entry(mst, ®istered_masters, node) { + ret |= mst->buses_supported; + } return ret; } diff --git a/programmer.h b/programmer.h index 913522b..54d5634 100644 --- a/programmer.h +++ b/programmer.h @@ -25,6 +25,7 @@ #define __PROGRAMMER_H__ 1 #include "flash.h" /* for chipaddr and flashctx */ +#include "list.h" enum programmer { #if CONFIG_INTERNAL == 1 @@ -691,6 +692,7 @@ struct par_master { }; int register_par_master(const struct par_master *mst, const enum chipbustype buses); struct registered_master { + struct list_head node; enum chipbustype buses_supported; union { struct par_master par; @@ -698,9 +700,8 @@ struct registered_master { struct opaque_master opaque; }; }; -extern struct registered_master registered_masters[]; -extern int registered_master_count; -int register_master(const struct registered_master *mst); +extern struct list_head registered_masters; +int register_master(struct registered_master *mst); /* serprog.c */ #if CONFIG_SERPROG == 1
Signed-off-by: Antony Pavlov <antonynpavlov@gmail.com> --- cli_classic.c | 13 +++--- flashrom.c | 1 - list.h | 130 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ programmer.c | 27 ++++++------ programmer.h | 7 ++-- 5 files changed, 153 insertions(+), 25 deletions(-)