[ovs-dev,1/2] dpif-netdev: Don't check if xcalloc() failed when creating meter.

Message ID 20180829004614.129668-1-jpettit@ovn.org
State New
Headers show
Series
  • [ovs-dev,1/2] dpif-netdev: Don't check if xcalloc() failed when creating meter.
Related show

Commit Message

Justin Pettit Aug. 29, 2018, 12:46 a.m.
xcalloc() can't return null.

Signed-off-by: Justin Pettit <jpettit@ovn.org>
---
 lib/dpif-netdev.c | 64 +++++++++++++++++++++++------------------------
 1 file changed, 31 insertions(+), 33 deletions(-)

Comments

Flavio Leitner Aug. 30, 2018, 2:17 p.m. | #1
On Tue, Aug 28, 2018 at 05:46:13PM -0700, Justin Pettit wrote:
> xcalloc() can't return null.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>
> ---

I couldn't find another instance of the problem and
this looks good.

Acked-by: Flavio Leitner <fbl@sysclose.org>
Ben Pfaff Aug. 30, 2018, 9 p.m. | #2
On Tue, Aug 28, 2018 at 05:46:13PM -0700, Justin Pettit wrote:
> xcalloc() can't return null.
> 
> Signed-off-by: Justin Pettit <jpettit@ovn.org>

Acked-by: Ben Pfaff <blp@ovn.org>
Justin Pettit Sept. 5, 2018, 6:43 p.m. | #3
> On Aug 30, 2018, at 7:17 AM, Flavio Leitner <fbl@sysclose.org> wrote:
> 
> On Tue, Aug 28, 2018 at 05:46:13PM -0700, Justin Pettit wrote:
>> xcalloc() can't return null.
>> 
>> Signed-off-by: Justin Pettit <jpettit@ovn.org>
>> ---
> 
> I couldn't find another instance of the problem and
> this looks good.
> 
> Acked-by: Flavio Leitner <fbl@sysclose.org>

Thanks.  I pushed this to master with yours and Ben's Acked-bys.

--Justin

Patch

diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c
index 807a462503ee..8b0b3745860b 100644
--- a/lib/dpif-netdev.c
+++ b/lib/dpif-netdev.c
@@ -5199,44 +5199,42 @@  dpif_netdev_meter_set(struct dpif *dpif, ofproto_meter_id meter_id,
     /* Allocate meter */
     meter = xzalloc(sizeof *meter
                     + config->n_bands * sizeof(struct dp_meter_band));
-    if (meter) {
-        meter->flags = config->flags;
-        meter->n_bands = config->n_bands;
-        meter->max_delta_t = 0;
-        meter->used = time_usec();
-
-        /* set up bands */
-        for (i = 0; i < config->n_bands; ++i) {
-            uint32_t band_max_delta_t;
-
-            /* Set burst size to a workable value if none specified. */
-            if (config->bands[i].burst_size == 0) {
-                config->bands[i].burst_size = config->bands[i].rate;
-            }
 
-            meter->bands[i].up = config->bands[i];
-            /* Convert burst size to the bucket units: */
-            /* pkts => 1/1000 packets, kilobits => bits. */
-            meter->bands[i].up.burst_size *= 1000;
-            /* Initialize bucket to empty. */
-            meter->bands[i].bucket = 0;
-
-            /* Figure out max delta_t that is enough to fill any bucket. */
-            band_max_delta_t
-                = meter->bands[i].up.burst_size / meter->bands[i].up.rate;
-            if (band_max_delta_t > meter->max_delta_t) {
-                meter->max_delta_t = band_max_delta_t;
-            }
+    meter->flags = config->flags;
+    meter->n_bands = config->n_bands;
+    meter->max_delta_t = 0;
+    meter->used = time_usec();
+
+    /* set up bands */
+    for (i = 0; i < config->n_bands; ++i) {
+        uint32_t band_max_delta_t;
+
+        /* Set burst size to a workable value if none specified. */
+        if (config->bands[i].burst_size == 0) {
+            config->bands[i].burst_size = config->bands[i].rate;
         }
 
-        meter_lock(dp, mid);
-        dp_delete_meter(dp, mid); /* Free existing meter, if any */
-        dp->meters[mid] = meter;
-        meter_unlock(dp, mid);
+        meter->bands[i].up = config->bands[i];
+        /* Convert burst size to the bucket units: */
+        /* pkts => 1/1000 packets, kilobits => bits. */
+        meter->bands[i].up.burst_size *= 1000;
+        /* Initialize bucket to empty. */
+        meter->bands[i].bucket = 0;
 
-        return 0;
+        /* Figure out max delta_t that is enough to fill any bucket. */
+        band_max_delta_t
+            = meter->bands[i].up.burst_size / meter->bands[i].up.rate;
+        if (band_max_delta_t > meter->max_delta_t) {
+            meter->max_delta_t = band_max_delta_t;
+        }
     }
-    return ENOMEM;
+
+    meter_lock(dp, mid);
+    dp_delete_meter(dp, mid); /* Free existing meter, if any */
+    dp->meters[mid] = meter;
+    meter_unlock(dp, mid);
+
+    return 0;
 }
 
 static int