Re: [libvirt] [PATCH 03/24] tests: hostdev: Declare count inside CHECK_LIST_COUNT()

[Andrea Bolognani]

On Mon, 2016-03-07 at 13:18 -0500, Laine Stump wrote:
> > +# define CHECK_LIST_COUNT(list, cnt)   
> >  \
> > +do {   
> >  \
> > +int count; 
> >  \
> > +if ((count = virPCIDeviceListCount(list)) != cnt) {
> >  \
> > +virReportError(VIR_ERR_INTERNAL_ERROR, 
> >  \
> > +   "Unexpected count of items in " #list ": %d, "  
> >  \
> > +   "expecting %zu", count, (size_t) cnt);  
> >  \
> > +goto cleanup;  
> >  \
> > +}  
> >  \
> > +} while (0)
> 
> The only suggestion I would have is to make "count" something less 
> common, so that you're less likely to end up causing a compiler warning 
> later if someone adds a variable called "count" to a function that uses 
> this macro.
> 
> Otherwise ACK.

I changed it to 'actualCount' before pushing, and explained the
reason for the change in the commit message.

Cheers.

-- 
Andrea Bolognani
Software Engineer - Virtualization Team

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 03/24] tests: hostdev: Declare count inside CHECK_LIST_COUNT()

[Laine Stump]

On 03/07/2016 12:24 PM, Andrea Bolognani wrote:

This variable is only used internally by the macro, so it's better to
move its definition inside the macro as well.
---
  tests/virhostdevtest.c | 31 +--
  1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index febb2c9..77b119b 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -37,13 +37,16 @@
  
  VIR_LOG_INIT("tests.hostdevtest");
  
-# define CHECK_LIST_COUNT(list, cnt)\

-if ((count = virPCIDeviceListCount(list)) != cnt) { \
-virReportError(VIR_ERR_INTERNAL_ERROR,  \
-   "Unexpected count of items in " #list ": %d, "   \
-   "expecting %zu", count, (size_t) cnt);   \
-goto cleanup;   \
-}
+# define CHECK_LIST_COUNT(list, cnt)\
+do {\
+int count;  \
+if ((count = virPCIDeviceListCount(list)) != cnt) { \
+virReportError(VIR_ERR_INTERNAL_ERROR,  \
+   "Unexpected count of items in " #list ": %d, "   \
+   "expecting %zu", count, (size_t) cnt);   \
+goto cleanup;   \
+}   \
+} while (0)


The only suggestion I would have is to make "count" something less 
common, so that you're less likely to end up causing a compiler warning 
later if someone adds a variable called "count" to a function that uses 
this macro.


Otherwise ACK.

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 03/24] tests: hostdev: Declare count inside CHECK_LIST_COUNT()

[Andrea Bolognani]

This variable is only used internally by the macro, so it's better to
move its definition inside the macro as well.
---
 tests/virhostdevtest.c | 31 +--
 1 file changed, 17 insertions(+), 14 deletions(-)

diff --git a/tests/virhostdevtest.c b/tests/virhostdevtest.c
index febb2c9..77b119b 100644
--- a/tests/virhostdevtest.c
+++ b/tests/virhostdevtest.c
@@ -37,13 +37,16 @@
 
 VIR_LOG_INIT("tests.hostdevtest");
 
-# define CHECK_LIST_COUNT(list, cnt)\
-if ((count = virPCIDeviceListCount(list)) != cnt) { \
-virReportError(VIR_ERR_INTERNAL_ERROR,  \
-   "Unexpected count of items in " #list ": %d, "   \
-   "expecting %zu", count, (size_t) cnt);   \
-goto cleanup;   \
-}
+# define CHECK_LIST_COUNT(list, cnt)\
+do {\
+int count;  \
+if ((count = virPCIDeviceListCount(list)) != cnt) { \
+virReportError(VIR_ERR_INTERNAL_ERROR,  \
+   "Unexpected count of items in " #list ": %d, "   \
+   "expecting %zu", count, (size_t) cnt);   \
+goto cleanup;   \
+}   \
+} while (0)
 
 # define TEST_STATE_DIR abs_builddir "/hostdevmgr"
 static const char *drv_name = "test_driver";
@@ -161,7 +164,7 @@ testVirHostdevPreparePCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count, inactive_count;
+int active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++)
  hostdevs[i]->managed = false;
@@ -220,7 +223,7 @@ testVirHostdevReAttachPCIHostdevs_unmanaged(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count, inactive_count;
+int active_count, inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != false) {
@@ -254,7 +257,7 @@ testVirHostdevPreparePCIHostdevs_managed(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count;
+int active_count;
 
 for (i = 0; i < nhostdevs; i++)
 hostdevs[i]->managed = true;
@@ -300,7 +303,7 @@ testVirHostdevReAttachPCIHostdevs_managed(const void 
*opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, active_count;
+int active_count;
 
 for (i = 0; i < nhostdevs; i++) {
 if (hostdevs[i]->managed != true) {
@@ -332,7 +335,7 @@ testVirHostdevDetachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, inactive_count;
+int inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -369,7 +372,7 @@ testVirHostdevReAttachPCINodeDevice(const void *opaque 
ATTRIBUTE_UNUSED)
 {
 int ret = -1;
 size_t i;
-int count, inactive_count;
+int inactive_count;
 
 for (i = 0; i < nhostdevs; i++) {
 inactive_count = virPCIDeviceListCount(mgr->inactivePCIHostdevs);
@@ -389,7 +392,7 @@ static int
 testVirHostdevUpdateActivePCIHostdevs(const void *opaque ATTRIBUTE_UNUSED)
 {
 int ret = -1;
-int count, active_count;
+int active_count;
 
 active_count = virPCIDeviceListCount(mgr->activePCIHostdevs);
 
-- 
2.5.0

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list