On 04/28/2013 08:31 PM, Igor Podlesny wrote:
---
  src/lib/util.c |   19 +++++++++----------
  1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/src/lib/util.c b/src/lib/util.c
index 8cc066b..420a574 100644
--- a/src/lib/util.c
+++ b/src/lib/util.c
@@ -46,18 +46,17 @@ static const char *unescapestr(char *const src)
        char *p1, *p2;
        int fl = 0;
- if (src == NULL)
-               return NULL;

I don't like this approach.

In current version, a reviewer don't have to read the whole function
for the case when src is NULL, and it's clear we don't do anything in that case.

In your version, he have to read it in it's entirety, and it's a bit less clear that we are
returning NULL for src == NULL case, plus it's an extra indentation level.

For the rest of the patches concerning unescapestr() -- do you really fix anything or it's just refactoring? If it's a pure refactoring, there should be a good reason for it,
which I missed from your comments.

-       
-       for (p1 = p2 = src; *p2; ++p2) {
-               if (*p2 == '\\' && !fl) {
-                       fl = 1;
-               } else {
-                       *p1++ = *p2;
-                       fl = 0;
+       if (src != NULL) {
+               for (p1 = p2 = src; *p2; ++p2) {
+                       if (*p2 == '\\' && !fl) {
+                               fl = 1;
+                       } else {
+                               *p1++ = *p2;
+                               fl = 0;
+                       }
                }
+               *p1 = 0;
        }
-       *p1 = 0;
return src;
  }

_______________________________________________
Devel mailing list
Devel@openvz.org
https://lists.openvz.org/mailman/listinfo/devel

Reply via email to