Message ID | 1340789229-13089-1-git-send-email-colin.king@canonical.com |
---|---|
State | Accepted |
Headers | show |
On 06/27/2012 05:27 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Output from xinput list can sometimes be re-ordered beween and > suspend and a resume, leading to fwts complaining that there are > differences between input devices even though it is all OK. The > simple workaround is to sort the output alphanumerically first > and then compare. > > We also filter out the pactl Latency information as this is > a new field that we don't want to compare because it is always > changing. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_hwinfo.c | 89 +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 74 insertions(+), 15 deletions(-) > > diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c > index 86d014e..e03df6a 100644 > --- a/src/lib/src/fwts_hwinfo.c > +++ b/src/lib/src/fwts_hwinfo.c > @@ -21,9 +21,13 @@ > > #include "fwts.h" > > +#define FWTS_HWINFO_LISTS_SAME (0) > +#define FWTS_HWINFO_LISTS_DIFFER (1) > +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) > + > /* > * fwts_hwinfo_get() > - * gather H/W information > + * gather H/W information > */ > int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > { > @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig); > fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); > fwts_pipe_exec("xinput list", &hwinfo->xinput); > - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); > - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl); > > return FWTS_OK; > } > @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework *fw, fwts_list *list) > */ > static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message) > { > - > fwts_log_info(fw, "%s configurations differ, before:", message); > fwts_hwinfo_list_dump(fw, l1); > fwts_log_info(fw, "versus after:"); > @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list > } > > /* > + * fwts_hwinfo_alpha_compare() > + * alphanumeric sort compare function > + */ > +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) > +{ > + return strcmp((char *)data1, (char*)data2); > +} > + > +/* > + * fwts_hwinfo_list_sort_alpha() > + * clone the contents of the given list and return an > + * alphanumerically sorted version of the list. This > + * list has cloned contents of the original, so it has > + * to be free'd with fwts_list_free(list, NULL) since > + * we don't want to free the cloned items and ruin the > + * original list > + */ > +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) > +{ > + fwts_list_link *item; > + fwts_list *sorted; > + > + if ((sorted = fwts_list_new()) == NULL) > + return NULL; > + > + fwts_list_foreach(item, list) { > + char *str = fwts_list_data(char *, item); > + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); > + } > + > + return sorted; > +} > + > +/* > * fwts_hwinfo_lists_differ() > - * check lists to see if their contents differ, return 1 for differ, 0 for match > + * check lists to see if their contents differ, return 1 for differ, 0 for match, > + * -1 for out of memory error > */ > static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > { > fwts_list_link *item1; > fwts_list_link *item2; > + > + fwts_list *sorted1, *sorted2; > > + /* Both null - both the same then */ > if ((l1 == NULL) && (l2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > + > + /* Either null and the other not? */ > if ((l1 == NULL) || (l2 == NULL)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + /* Different length, then they differ */ > if (fwts_list_len(l1) != fwts_list_len(l2)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + > + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { > + fwts_list_free(sorted1, NULL); > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + } > > - item1 = fwts_list_head(l1); > - item2 = fwts_list_head(l2); > + item1 = fwts_list_head(sorted1); > + item2 = fwts_list_head(sorted2); > > while ((item1 != NULL) && (item2 != NULL)) { > char *str1 = fwts_list_data(char *, item1); > char *str2 = fwts_list_data(char *, item2); > > - if (strcmp(str1, str2)) > - return 1; > + if (strcmp(str1, str2)) { > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + return FWTS_HWINFO_LISTS_DIFFER; > + } > item1 = fwts_list_next(item1); > item2 = fwts_list_next(item2); > } > > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + > if ((item1 == NULL) && (item2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > else > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > } > > /* > @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > */ > static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message, int *differences) > { > - if (fwts_hwinfo_lists_differ(l1, l2)) { > + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { > (*differences)++; > fwts_hwinfo_lists_dump(fw, l1, l2, message); > } > Gave this a spin on my machine with an Apple Magic Trackpad, did see one device reorder in xinput -list (everything still functional) and no complaints from fwts. Tested-by: Chris Van Hoof <vanhoof@canonical.com>
On 06/27/2012 05:27 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Output from xinput list can sometimes be re-ordered beween and > suspend and a resume, leading to fwts complaining that there are > differences between input devices even though it is all OK. The > simple workaround is to sort the output alphanumerically first > and then compare. > > We also filter out the pactl Latency information as this is > a new field that we don't want to compare because it is always > changing. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_hwinfo.c | 89 +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 74 insertions(+), 15 deletions(-) > > diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c > index 86d014e..e03df6a 100644 > --- a/src/lib/src/fwts_hwinfo.c > +++ b/src/lib/src/fwts_hwinfo.c > @@ -21,9 +21,13 @@ > > #include "fwts.h" > > +#define FWTS_HWINFO_LISTS_SAME (0) > +#define FWTS_HWINFO_LISTS_DIFFER (1) > +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) > + > /* > * fwts_hwinfo_get() > - * gather H/W information > + * gather H/W information > */ > int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > { > @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig); > fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); > fwts_pipe_exec("xinput list", &hwinfo->xinput); > - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); > - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl); > > return FWTS_OK; > } > @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework *fw, fwts_list *list) > */ > static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message) > { > - > fwts_log_info(fw, "%s configurations differ, before:", message); > fwts_hwinfo_list_dump(fw, l1); > fwts_log_info(fw, "versus after:"); > @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list > } > > /* > + * fwts_hwinfo_alpha_compare() > + * alphanumeric sort compare function > + */ > +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) > +{ > + return strcmp((char *)data1, (char*)data2); > +} > + > +/* > + * fwts_hwinfo_list_sort_alpha() > + * clone the contents of the given list and return an > + * alphanumerically sorted version of the list. This > + * list has cloned contents of the original, so it has > + * to be free'd with fwts_list_free(list, NULL) since > + * we don't want to free the cloned items and ruin the > + * original list > + */ > +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) > +{ > + fwts_list_link *item; > + fwts_list *sorted; > + > + if ((sorted = fwts_list_new()) == NULL) > + return NULL; > + > + fwts_list_foreach(item, list) { > + char *str = fwts_list_data(char *, item); > + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); > + } > + > + return sorted; > +} > + > +/* > * fwts_hwinfo_lists_differ() > - * check lists to see if their contents differ, return 1 for differ, 0 for match > + * check lists to see if their contents differ, return 1 for differ, 0 for match, > + * -1 for out of memory error > */ > static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > { > fwts_list_link *item1; > fwts_list_link *item2; > + > + fwts_list *sorted1, *sorted2; > > + /* Both null - both the same then */ > if ((l1 == NULL) && (l2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > + > + /* Either null and the other not? */ > if ((l1 == NULL) || (l2 == NULL)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + /* Different length, then they differ */ > if (fwts_list_len(l1) != fwts_list_len(l2)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + > + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { > + fwts_list_free(sorted1, NULL); > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + } > > - item1 = fwts_list_head(l1); > - item2 = fwts_list_head(l2); > + item1 = fwts_list_head(sorted1); > + item2 = fwts_list_head(sorted2); > > while ((item1 != NULL) && (item2 != NULL)) { > char *str1 = fwts_list_data(char *, item1); > char *str2 = fwts_list_data(char *, item2); > > - if (strcmp(str1, str2)) > - return 1; > + if (strcmp(str1, str2)) { > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + return FWTS_HWINFO_LISTS_DIFFER; > + } > item1 = fwts_list_next(item1); > item2 = fwts_list_next(item2); > } > > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + > if ((item1 == NULL) && (item2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > else > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > } > > /* > @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > */ > static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message, int *differences) > { > - if (fwts_hwinfo_lists_differ(l1, l2)) { > + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { > (*differences)++; > fwts_hwinfo_lists_dump(fw, l1, l2, message); > } > Acked-by: Alex Hung <alex.hung@canonical.com>
On 28/06/12 00:58, Chris Van Hoof wrote: > On 06/27/2012 05:27 AM, Colin King wrote: >> From: Colin Ian King <colin.king@canonical.com> >> >> Output from xinput list can sometimes be re-ordered beween and >> suspend and a resume, leading to fwts complaining that there are >> differences between input devices even though it is all OK. The >> simple workaround is to sort the output alphanumerically first >> and then compare. >> >> We also filter out the pactl Latency information as this is >> a new field that we don't want to compare because it is always >> changing. >> >> Signed-off-by: Colin Ian King <colin.king@canonical.com> >> --- >> src/lib/src/fwts_hwinfo.c | 89 >> +++++++++++++++++++++++++++++++++++++-------- >> 1 file changed, 74 insertions(+), 15 deletions(-) >> >> diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c >> index 86d014e..e03df6a 100644 >> --- a/src/lib/src/fwts_hwinfo.c >> +++ b/src/lib/src/fwts_hwinfo.c >> @@ -21,9 +21,13 @@ >> >> #include "fwts.h" >> >> +#define FWTS_HWINFO_LISTS_SAME (0) >> +#define FWTS_HWINFO_LISTS_DIFFER (1) >> +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) >> + >> /* >> * fwts_hwinfo_get() >> - * gather H/W information >> + * gather H/W information >> */ >> int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) >> { >> @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo >> *hwinfo) >> fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", >> &hwinfo->hciconfig); >> fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); >> fwts_pipe_exec("xinput list", &hwinfo->xinput); >> - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); >> - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); >> + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", >> &hwinfo->pactl); >> + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", >> &hwinfo->pactl); >> >> return FWTS_OK; >> } >> @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework >> *fw, fwts_list *list) >> */ >> static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list >> *l1, fwts_list *l2, char *message) >> { >> - >> fwts_log_info(fw, "%s configurations differ, before:", message); >> fwts_hwinfo_list_dump(fw, l1); >> fwts_log_info(fw, "versus after:"); >> @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework >> *fw, fwts_list *l1, fwts_list >> } >> >> /* >> + * fwts_hwinfo_alpha_compare() >> + * alphanumeric sort compare function >> + */ >> +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) >> +{ >> + return strcmp((char *)data1, (char*)data2); >> +} >> + >> +/* >> + * fwts_hwinfo_list_sort_alpha() >> + * clone the contents of the given list and return an >> + * alphanumerically sorted version of the list. This >> + * list has cloned contents of the original, so it has >> + * to be free'd with fwts_list_free(list, NULL) since >> + * we don't want to free the cloned items and ruin the >> + * original list >> + */ >> +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) >> +{ >> + fwts_list_link *item; >> + fwts_list *sorted; >> + >> + if ((sorted = fwts_list_new()) == NULL) >> + return NULL; >> + >> + fwts_list_foreach(item, list) { >> + char *str = fwts_list_data(char *, item); >> + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); >> + } >> + >> + return sorted; >> +} >> + >> +/* >> * fwts_hwinfo_lists_differ() >> - * check lists to see if their contents differ, return 1 for >> differ, 0 for match >> + * check lists to see if their contents differ, return 1 for >> differ, 0 for match, >> + * -1 for out of memory error >> */ >> static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) >> { >> fwts_list_link *item1; >> fwts_list_link *item2; >> + >> + fwts_list *sorted1, *sorted2; >> >> + /* Both null - both the same then */ >> if ((l1 == NULL) && (l2 == NULL)) >> - return 0; >> + return FWTS_HWINFO_LISTS_SAME; >> + >> + /* Either null and the other not? */ >> if ((l1 == NULL) || (l2 == NULL)) >> - return 1; >> + return FWTS_HWINFO_LISTS_DIFFER; >> + >> + /* Different length, then they differ */ >> if (fwts_list_len(l1) != fwts_list_len(l2)) >> - return 1; >> + return FWTS_HWINFO_LISTS_DIFFER; >> + >> + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) >> + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; >> + >> + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { >> + fwts_list_free(sorted1, NULL); >> + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; >> + } >> >> - item1 = fwts_list_head(l1); >> - item2 = fwts_list_head(l2); >> + item1 = fwts_list_head(sorted1); >> + item2 = fwts_list_head(sorted2); >> >> while ((item1 != NULL) && (item2 != NULL)) { >> char *str1 = fwts_list_data(char *, item1); >> char *str2 = fwts_list_data(char *, item2); >> >> - if (strcmp(str1, str2)) >> - return 1; >> + if (strcmp(str1, str2)) { >> + fwts_list_free(sorted1, NULL); >> + fwts_list_free(sorted2, NULL); >> + return FWTS_HWINFO_LISTS_DIFFER; >> + } >> item1 = fwts_list_next(item1); >> item2 = fwts_list_next(item2); >> } >> >> + fwts_list_free(sorted1, NULL); >> + fwts_list_free(sorted2, NULL); >> + >> if ((item1 == NULL) && (item2 == NULL)) >> - return 0; >> + return FWTS_HWINFO_LISTS_SAME; >> else >> - return 1; >> + return FWTS_HWINFO_LISTS_DIFFER; >> } >> >> /* >> @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, >> fwts_list *l2) >> */ >> static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list >> *l1, fwts_list *l2, char *message, int *differences) >> { >> - if (fwts_hwinfo_lists_differ(l1, l2)) { >> + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { >> (*differences)++; >> fwts_hwinfo_lists_dump(fw, l1, l2, message); >> } >> > > Gave this a spin on my machine with an Apple Magic Trackpad, did see one > device reorder in xinput -list (everything still functional) and no > complaints from fwts. > > Tested-by: Chris Van Hoof <vanhoof@canonical.com> > Thanks for testing, I didn't have any H/W to exercise this properly.
On Wed, Jun 27, 2012 at 5:27 PM, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > Output from xinput list can sometimes be re-ordered beween and > suspend and a resume, leading to fwts complaining that there are > differences between input devices even though it is all OK. The > simple workaround is to sort the output alphanumerically first > and then compare. > > We also filter out the pactl Latency information as this is > a new field that we don't want to compare because it is always > changing. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_hwinfo.c | 89 +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 74 insertions(+), 15 deletions(-) > > diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c > index 86d014e..e03df6a 100644 > --- a/src/lib/src/fwts_hwinfo.c > +++ b/src/lib/src/fwts_hwinfo.c > @@ -21,9 +21,13 @@ > > #include "fwts.h" > > +#define FWTS_HWINFO_LISTS_SAME (0) > +#define FWTS_HWINFO_LISTS_DIFFER (1) > +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) > + > /* > * fwts_hwinfo_get() > - * gather H/W information > + * gather H/W information > */ > int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > { > @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig); > fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); > fwts_pipe_exec("xinput list", &hwinfo->xinput); > - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); > - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl); > > return FWTS_OK; > } > @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework *fw, fwts_list *list) > */ > static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message) > { > - > fwts_log_info(fw, "%s configurations differ, before:", message); > fwts_hwinfo_list_dump(fw, l1); > fwts_log_info(fw, "versus after:"); > @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list > } > > /* > + * fwts_hwinfo_alpha_compare() > + * alphanumeric sort compare function > + */ > +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) > +{ > + return strcmp((char *)data1, (char*)data2); > +} > + > +/* > + * fwts_hwinfo_list_sort_alpha() > + * clone the contents of the given list and return an > + * alphanumerically sorted version of the list. This > + * list has cloned contents of the original, so it has > + * to be free'd with fwts_list_free(list, NULL) since > + * we don't want to free the cloned items and ruin the > + * original list > + */ > +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) > +{ > + fwts_list_link *item; > + fwts_list *sorted; > + > + if ((sorted = fwts_list_new()) == NULL) > + return NULL; > + > + fwts_list_foreach(item, list) { > + char *str = fwts_list_data(char *, item); > + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); > + } > + > + return sorted; > +} > + > +/* > * fwts_hwinfo_lists_differ() > - * check lists to see if their contents differ, return 1 for differ, 0 for match > + * check lists to see if their contents differ, return 1 for differ, 0 for match, > + * -1 for out of memory error > */ > static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > { > fwts_list_link *item1; > fwts_list_link *item2; > + > + fwts_list *sorted1, *sorted2; > > + /* Both null - both the same then */ > if ((l1 == NULL) && (l2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > + > + /* Either null and the other not? */ > if ((l1 == NULL) || (l2 == NULL)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + /* Different length, then they differ */ > if (fwts_list_len(l1) != fwts_list_len(l2)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + > + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { > + fwts_list_free(sorted1, NULL); > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + } > > - item1 = fwts_list_head(l1); > - item2 = fwts_list_head(l2); > + item1 = fwts_list_head(sorted1); > + item2 = fwts_list_head(sorted2); > > while ((item1 != NULL) && (item2 != NULL)) { > char *str1 = fwts_list_data(char *, item1); > char *str2 = fwts_list_data(char *, item2); > > - if (strcmp(str1, str2)) > - return 1; > + if (strcmp(str1, str2)) { > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + return FWTS_HWINFO_LISTS_DIFFER; > + } > item1 = fwts_list_next(item1); > item2 = fwts_list_next(item2); > } > > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + > if ((item1 == NULL) && (item2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > else > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > } > > /* > @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > */ > static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message, int *differences) > { > - if (fwts_hwinfo_lists_differ(l1, l2)) { > + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { > (*differences)++; > fwts_hwinfo_lists_dump(fw, l1, l2, message); > } > -- > 1.7.10.4 > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
On 06/27/2012 05:27 PM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > Output from xinput list can sometimes be re-ordered beween and > suspend and a resume, leading to fwts complaining that there are > differences between input devices even though it is all OK. The > simple workaround is to sort the output alphanumerically first > and then compare. > > We also filter out the pactl Latency information as this is > a new field that we don't want to compare because it is always > changing. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_hwinfo.c | 89 +++++++++++++++++++++++++++++++++++++-------- > 1 file changed, 74 insertions(+), 15 deletions(-) > > diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c > index 86d014e..e03df6a 100644 > --- a/src/lib/src/fwts_hwinfo.c > +++ b/src/lib/src/fwts_hwinfo.c > @@ -21,9 +21,13 @@ > > #include "fwts.h" > > +#define FWTS_HWINFO_LISTS_SAME (0) > +#define FWTS_HWINFO_LISTS_DIFFER (1) > +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) > + > /* > * fwts_hwinfo_get() > - * gather H/W information > + * gather H/W information > */ > int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > { > @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) > fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig); > fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); > fwts_pipe_exec("xinput list", &hwinfo->xinput); > - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); > - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl); > + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl); > > return FWTS_OK; > } > @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework *fw, fwts_list *list) > */ > static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message) > { > - > fwts_log_info(fw, "%s configurations differ, before:", message); > fwts_hwinfo_list_dump(fw, l1); > fwts_log_info(fw, "versus after:"); > @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list > } > > /* > + * fwts_hwinfo_alpha_compare() > + * alphanumeric sort compare function > + */ > +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) > +{ > + return strcmp((char *)data1, (char*)data2); > +} > + > +/* > + * fwts_hwinfo_list_sort_alpha() > + * clone the contents of the given list and return an > + * alphanumerically sorted version of the list. This > + * list has cloned contents of the original, so it has > + * to be free'd with fwts_list_free(list, NULL) since > + * we don't want to free the cloned items and ruin the > + * original list > + */ > +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) > +{ > + fwts_list_link *item; > + fwts_list *sorted; > + > + if ((sorted = fwts_list_new()) == NULL) > + return NULL; > + > + fwts_list_foreach(item, list) { > + char *str = fwts_list_data(char *, item); > + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); > + } > + > + return sorted; > +} > + > +/* > * fwts_hwinfo_lists_differ() > - * check lists to see if their contents differ, return 1 for differ, 0 for match > + * check lists to see if their contents differ, return 1 for differ, 0 for match, > + * -1 for out of memory error > */ > static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > { > fwts_list_link *item1; > fwts_list_link *item2; > + > + fwts_list *sorted1, *sorted2; > > + /* Both null - both the same then */ > if ((l1 == NULL) && (l2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > + > + /* Either null and the other not? */ > if ((l1 == NULL) || (l2 == NULL)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + /* Different length, then they differ */ > if (fwts_list_len(l1) != fwts_list_len(l2)) > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > + > + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + > + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { > + fwts_list_free(sorted1, NULL); > + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; > + } > > - item1 = fwts_list_head(l1); > - item2 = fwts_list_head(l2); > + item1 = fwts_list_head(sorted1); > + item2 = fwts_list_head(sorted2); > > while ((item1 != NULL) && (item2 != NULL)) { > char *str1 = fwts_list_data(char *, item1); > char *str2 = fwts_list_data(char *, item2); > > - if (strcmp(str1, str2)) > - return 1; > + if (strcmp(str1, str2)) { > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + return FWTS_HWINFO_LISTS_DIFFER; > + } > item1 = fwts_list_next(item1); > item2 = fwts_list_next(item2); > } > > + fwts_list_free(sorted1, NULL); > + fwts_list_free(sorted2, NULL); > + > if ((item1 == NULL) && (item2 == NULL)) > - return 0; > + return FWTS_HWINFO_LISTS_SAME; > else > - return 1; > + return FWTS_HWINFO_LISTS_DIFFER; > } > > /* > @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) > */ > static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message, int *differences) > { > - if (fwts_hwinfo_lists_differ(l1, l2)) { > + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { > (*differences)++; > fwts_hwinfo_lists_dump(fw, l1, l2, message); > } > Acked-by: Ivan Hu<ivan.hu@canonical.com>
diff --git a/src/lib/src/fwts_hwinfo.c b/src/lib/src/fwts_hwinfo.c index 86d014e..e03df6a 100644 --- a/src/lib/src/fwts_hwinfo.c +++ b/src/lib/src/fwts_hwinfo.c @@ -21,9 +21,13 @@ #include "fwts.h" +#define FWTS_HWINFO_LISTS_SAME (0) +#define FWTS_HWINFO_LISTS_DIFFER (1) +#define FWTS_HWINFO_LISTS_OUT_OF_MEMORY (-1) + /* * fwts_hwinfo_get() - * gather H/W information + * gather H/W information */ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) { @@ -34,8 +38,8 @@ int fwts_hwinfo_get(fwts_framework *fw, fwts_hwinfo *hwinfo) fwts_pipe_exec("hciconfig -a | grep -A2 '^\\w", &hwinfo->hciconfig); fwts_pipe_exec("lspci | grep VGA", &hwinfo->videocard); fwts_pipe_exec("xinput list", &hwinfo->xinput); - fwts_pipe_exec("pactl list | grep Sink", &hwinfo->pactl); - fwts_pipe_exec("pactl list | grep Source", &hwinfo->pactl); + fwts_pipe_exec("pactl list | grep Sink | grep -v Latency", &hwinfo->pactl); + fwts_pipe_exec("pactl list | grep Source | grep -v Latency", &hwinfo->pactl); return FWTS_OK; } @@ -64,7 +68,6 @@ static void fwts_hwinfo_list_dump(fwts_framework *fw, fwts_list *list) */ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message) { - fwts_log_info(fw, "%s configurations differ, before:", message); fwts_hwinfo_list_dump(fw, l1); fwts_log_info(fw, "versus after:"); @@ -73,38 +76,94 @@ static void fwts_hwinfo_lists_dump(fwts_framework *fw, fwts_list *l1, fwts_list } /* + * fwts_hwinfo_alpha_compare() + * alphanumeric sort compare function + */ +static int fwts_hwinfo_alpha_compare(void *data1, void *data2) +{ + return strcmp((char *)data1, (char*)data2); +} + +/* + * fwts_hwinfo_list_sort_alpha() + * clone the contents of the given list and return an + * alphanumerically sorted version of the list. This + * list has cloned contents of the original, so it has + * to be free'd with fwts_list_free(list, NULL) since + * we don't want to free the cloned items and ruin the + * original list + */ +static fwts_list *fwts_hwinfo_list_sort_alpha(fwts_list *list) +{ + fwts_list_link *item; + fwts_list *sorted; + + if ((sorted = fwts_list_new()) == NULL) + return NULL; + + fwts_list_foreach(item, list) { + char *str = fwts_list_data(char *, item); + fwts_list_add_ordered(sorted, str, fwts_hwinfo_alpha_compare); + } + + return sorted; +} + +/* * fwts_hwinfo_lists_differ() - * check lists to see if their contents differ, return 1 for differ, 0 for match + * check lists to see if their contents differ, return 1 for differ, 0 for match, + * -1 for out of memory error */ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) { fwts_list_link *item1; fwts_list_link *item2; + + fwts_list *sorted1, *sorted2; + /* Both null - both the same then */ if ((l1 == NULL) && (l2 == NULL)) - return 0; + return FWTS_HWINFO_LISTS_SAME; + + /* Either null and the other not? */ if ((l1 == NULL) || (l2 == NULL)) - return 1; + return FWTS_HWINFO_LISTS_DIFFER; + + /* Different length, then they differ */ if (fwts_list_len(l1) != fwts_list_len(l2)) - return 1; + return FWTS_HWINFO_LISTS_DIFFER; + + if ((sorted1 = fwts_hwinfo_list_sort_alpha(l1)) == NULL) + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; + + if ((sorted2 = fwts_hwinfo_list_sort_alpha(l2)) == NULL) { + fwts_list_free(sorted1, NULL); + return FWTS_HWINFO_LISTS_OUT_OF_MEMORY; + } - item1 = fwts_list_head(l1); - item2 = fwts_list_head(l2); + item1 = fwts_list_head(sorted1); + item2 = fwts_list_head(sorted2); while ((item1 != NULL) && (item2 != NULL)) { char *str1 = fwts_list_data(char *, item1); char *str2 = fwts_list_data(char *, item2); - if (strcmp(str1, str2)) - return 1; + if (strcmp(str1, str2)) { + fwts_list_free(sorted1, NULL); + fwts_list_free(sorted2, NULL); + return FWTS_HWINFO_LISTS_DIFFER; + } item1 = fwts_list_next(item1); item2 = fwts_list_next(item2); } + fwts_list_free(sorted1, NULL); + fwts_list_free(sorted2, NULL); + if ((item1 == NULL) && (item2 == NULL)) - return 0; + return FWTS_HWINFO_LISTS_SAME; else - return 1; + return FWTS_HWINFO_LISTS_DIFFER; } /* @@ -113,7 +172,7 @@ static int fwts_hwinfo_lists_differ(fwts_list *l1, fwts_list *l2) */ static void fwts_hwinfo_lists_compare(fwts_framework *fw, fwts_list *l1, fwts_list *l2, char *message, int *differences) { - if (fwts_hwinfo_lists_differ(l1, l2)) { + if (fwts_hwinfo_lists_differ(l1, l2) == FWTS_HWINFO_LISTS_DIFFER) { (*differences)++; fwts_hwinfo_lists_dump(fw, l1, l2, message); }