On 08/14/2016 10:52 PM, Konrad Rzeszutek Wilk wrote:
Specifically the code that is looking up f->old_addr - which
can be in its own routine instead of having it part of prepare_payload.

No functional change.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.w...@oracle.com>

---
Cc: Ross Lagerwall <ross.lagerw...@citrix.com>

v3: First submission.
---
 xen/common/livepatch.c | 43 ++++++++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 17 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 28a400f..cbfeac1 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -232,6 +232,29 @@ static const char *livepatch_symbols_lookup(unsigned long 
addr,
     return n;
 }

+static int lookup_symbol(struct livepatch_func *f, struct livepatch_elf *elf)
+{
+    if ( f->old_addr )
+        return 0;
+
+    /* Lookup function's old address if not already resolved. */

This comment needs to move further up for it to be correct, since at this point we know that it is not yet resolved. How about putting it at the top of the function?

+    f->old_addr = (void *)symbols_lookup_by_name(f->name);
+    if ( !f->old_addr )
+    {
+        f->old_addr = (void *)livepatch_symbols_lookup_by_name(f->name);
+        if ( !f->old_addr )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Could not resolve old address of 
%s\n",
+                    elf->name, f->name);
+            return -ENOENT;
+        }
+    }
+    dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Resolved old address %s => %p\n",
+            elf->name, f->name, f->old_addr);
+
+    return 0;
+}
+
 static struct payload *find_payload(const char *name)
 {
     struct payload *data, *found = NULL;
@@ -510,23 +533,9 @@ static int prepare_payload(struct payload *payload,
         if ( rc )
             return rc;

-        /* Lookup function's old address if not already resolved. */
-        if ( !f->old_addr )
-        {
-            f->old_addr = (void *)symbols_lookup_by_name(f->name);
-            if ( !f->old_addr )
-            {
-                f->old_addr = (void 
*)livepatch_symbols_lookup_by_name(f->name);
-                if ( !f->old_addr )
-                {
-                    dprintk(XENLOG_ERR, LIVEPATCH "%s: Could not resolve old 
address of %s\n",
-                            elf->name, f->name);
-                    return -ENOENT;
-                }
-            }
-            dprintk(XENLOG_DEBUG, LIVEPATCH "%s: Resolved old address %s => 
%p\n",
-                    elf->name, f->name, f->old_addr);
-        }
+        rc = lookup_symbol(f, elf);
+        if ( rc )
+            return rc;

Can you give the function a less generic name? E.g. resolve_old_address

--
Ross Lagerwall

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Reply via email to