On Thu, 20 Apr 2017, Thomas Gleixner wrote:

On Wed, 19 Apr 2017, Vikas Shivappa wrote:

                }

The resulting loop is just horrible to read. We can do better than
that. Patch below.

Thanks,

        tglx
8<-------------

--- a/arch/x86/kernel/cpu/intel_rdt_schemata.c
+++ b/arch/x86/kernel/cpu/intel_rdt_schemata.c
@@ -187,6 +187,17 @@ static int update_domains(struct rdt_res
        return 0;
}

+static int rdtgroup_parse_resource(char *resname, char *tok, int closid)
+{
+       struct rdt_resource *r;
+
+       for_each_enabled_rdt_resource(r) {
+               if (!strcmp(resname, r->name) && closid < r->num_closid)
+                       return parse_line(tok, r);
+       }
+       return -EINVAL;
+}
+
ssize_t rdtgroup_schemata_write(struct kernfs_open_file *of,
                                char *buf, size_t nbytes, loff_t off)
{
@@ -209,9 +220,10 @@ ssize_t rdtgroup_schemata_write(struct k

        closid = rdtgrp->closid;

-       for_each_enabled_rdt_resource(r)
+       for_each_enabled_rdt_resource(r) {
                list_for_each_entry(dom, &r->domains, list)
                        dom->have_new_ctrl = false;
+       }

        while ((tok = strsep(&buf, "\n")) != NULL) {
                resname = strsep(&tok, ":");
@@ -219,19 +231,9 @@ ssize_t rdtgroup_schemata_write(struct k
                        ret = -EINVAL;
                        goto out;
                }
-               for_each_enabled_rdt_resource(r) {
-                       if (!strcmp(strim(resname), r->name) &&
-                           closid < r->num_closid) {
-                               ret = parse_line(tok, r);
-                               if (ret)
-                                       goto out;
-                               break;
-                       }
-               }
-               if (!r->name) {
-                       ret = -EINVAL;
+               ret = rdtgroup_parse_resource(strim(resname), tok, closid);
+               if (ret)
                        goto out;
-               }
        }

Ok agree its better way. I was trying to add a NULL which Tony already said was Nacked, did not really want to use a pointer which may be out of bounds although we dont check the contents.

Thanks,
Vikas

Reply via email to