[PATCH] phyp: sed cleanups

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



* src/phyp/phyp_driver.c (phypNumDomainsGeneric): Avoid glob
collision by quoting sed argument.
(phypDomainSetCPU): Avoid non-portable \+ in sed.
(phypGetVIOSPartitionID, phypDiskType, phypListDomainsGeneric)
(phypListDefinedDomains): Micro-optimize anchored substitutions.
---

> > +    if (system_type == HMC) {
> > +        if (virAsprintf(&cmd,
> > +                        "lssyscfg -r lpar -m %s -F lpar_id,state %s |grep -c "
> > +                        "^[0-9]*", managed_system, state) < 0) {
> Ouch - a pre-existing bug.  Since this command is being fed to a shell,
> you need to properly quote the regex being fed to grep, so that it
> doesn't get interpreted as a glob matching a literal file (such as ^0)
> that happens to be in the same directory.  That should be fixed in an
> independent patch.

Like this one :)

> > +             "lssyscfg -r lpar -m %s -F lpar_id,state %s | sed -e 's/,.*$//g'",
> Here, a minor optimization - you don't need the g flag to that sed s///
> expression, since the regex is anchored to the end of the string (you
> can't replace more than one anchored expression per line).  But
> independent of this patch.

While I was touching the file...

 src/phyp/phyp_driver.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index 0f4bc20..f8d12e7 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -886,7 +886,7 @@ phypGetVIOSPartitionID(virConnectPtr conn)

     if (virAsprintf(&cmd,
                     "lssyscfg -m %s -r lpar -F lpar_id,lpar_env|grep "
-                    "vioserver|sed -s 's/,.*$//g'", managed_system) < 0) {
+                    "vioserver|sed -s 's/,.*$//'", managed_system) < 0) {
         virReportOOMError();
         goto err;
     }
@@ -925,7 +925,7 @@ phypDiskType(virConnectPtr conn, char *backing_device)

     if (virAsprintf(&cmd,
                     "viosvrcmd -m %s -p %d -c \"lssp -field name type "
-                    "-fmt , -all|grep %s|sed -e 's/^.*,//g'\"",
+                    "-fmt , -all|grep %s|sed -e 's/^.*,//'\"",
                     managed_system, vios_id, backing_device) < 0) {
         virReportOOMError();
         goto cleanup;
@@ -983,7 +983,7 @@ phypNumDomainsGeneric(virConnectPtr conn, unsigned int type)

     if (virAsprintf(&cmd,
                     "lssyscfg -r lpar -m %s -F lpar_id,state %s |grep -c "
-                    "^[0-9]*", managed_system, state) < 0) {
+                    "'^[0-9]*'", managed_system, state) < 0) {
         virReportOOMError();
         goto err;
     }
@@ -1051,7 +1051,7 @@ phypListDomainsGeneric(virConnectPtr conn, int *ids, int nids,

     if (virAsprintf
         (&cmd,
-         "lssyscfg -r lpar -m %s -F lpar_id,state %s | sed -e 's/,.*$//g'",
+         "lssyscfg -r lpar -m %s -F lpar_id,state %s | sed -e 's/,.*$//'",
          managed_system, state) < 0) {
         virReportOOMError();
         goto err;
@@ -1115,7 +1115,7 @@ phypListDefinedDomains(virConnectPtr conn, char **const names, int nnames)
     if (virAsprintf
         (&cmd,
          "lssyscfg -r lpar -m %s -F name,state | grep \"Not Activated\" | "
-         "sed -e 's/,.*$//g'", managed_system) < 0) {
+         "sed -e 's/,.*$//'", managed_system) < 0) {
         virReportOOMError();
         goto err;
     }
@@ -1539,7 +1539,7 @@ phypDomainSetCPU(virDomainPtr dom, unsigned int nvcpus)
     if (virAsprintf
         (&cmd,
          "chhwres -r proc -m %s --id %d -o %c --procunits %d 2>&1 |sed"
-         "-e 's/^.*\\([0-9]\\+.[0-9]\\+\\).*$/\\1/g'",
+         "-e 's/^.*\\([0-9][0-9]*.[0-9][0-9]*\\).*$/\\1/'",
          managed_system, dom->id, operation, amount) < 0) {
         virReportOOMError();
         goto err;
-- 
1.7.0.1

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


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]