Re: [Libguestfs] [PATCH v2 1/2] Modify public function: analyze_line, make it more flexible

[Cao jin]

Hi Pino

在 2015年06月17日 19:15, Pino Toscano 写道:
On Wednesday 17 June 2015 16:44:07 Cao jin wrote:
Mofify the function from fixed dilemiter to variabler. So,
it could be used in more APIs later. Also modified the existed caller

Not to sound too overly pedantic on English (especially that I'm not
a native speaker myself), but this needs to be slightly improved:


Sorry for making you have that feeling, didn`t mean to make it sound 
that way. Because I am not native English speaker too, and also poor 
vocabulary, so maybe sometimes, when I want to describe something 
clearly, wish make it easier to be understood, lots of redundant or 
exaggerated words or some words like that are used, but actually I did 
not realized:(

   Modify the function from a fixed delimiter to a variable. So,
   it can be used in more APIs later. Also modified the existing
   callers.


Yes, your improvement is better.

Also, it is not a public function, just an internal one.


Yup, it is static, not public over the project, my original meaning is, 
it is "public" within btrfs.c, maybe other btrfs APIs will use it later. 
sorry for my bad description:)


Signed-off-by: Cao jin <caoj.f...@cn.fujitsu.com>
---
  daemon/btrfs.c | 14 +++++++-------
  1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/daemon/btrfs.c b/daemon/btrfs.c
index 39392f7..caa28ca 100644
--- a/daemon/btrfs.c
+++ b/daemon/btrfs.c
@@ -853,11 +853,10 @@ do_btrfs_fsck (const char *device, int64_t superblock, 
int repair)
   * returns the next position following \n.
   */
  static char *
-analyze_line (char *line, char **key, char **value)
+analyze_line (char *line, char **key, char **value, char delimiter)
  {
    char *p = line;
    char *next = NULL;
-  char delimiter = ':';
    char *del_pos = NULL;

    if (!line || *line == '\0') {
@@ -964,7 +963,7 @@ do_btrfs_subvolume_show (const char *subvolume)
     *                                 snapshots/test3
     *
     */
-  p = analyze_line(out, &key, &value);
+  p = analyze_line(out, &key, &value, ':');
    if (!p) {
      reply_with_error ("truncated output: %s", out);
      return NULL;
@@ -984,7 +983,7 @@ do_btrfs_subvolume_show (const char *subvolume)
    }

    /* Read the lines and split into "key: value". */
-  p = analyze_line(p, &key, &value);
+  p = analyze_line(p, &key, &value, ':');
    while (key) {
      /* snapshot is special, see the output above */
      if (STREQLEN (key, "Snapshot(s)", sizeof ("Snapshot(s)") - 1)) {
@@ -994,7 +993,7 @@ do_btrfs_subvolume_show (const char *subvolume)
        if (add_string (&ret, key) == -1)
          return NULL;

-      p = analyze_line(p, &key, &value);
+      p = analyze_line(p, &key, &value, ':');

        while (key && !value) {
            ss = realloc (ss, ss_len + strlen (key) + 1);
@@ -1008,7 +1007,7 @@ do_btrfs_subvolume_show (const char *subvolume)
            ss_len += strlen (key);
            ss[ss_len] = '\0';

-          p = analyze_line(p, &key, &value);
+          p = analyze_line(p, &key, &value, ':');
        }

        if (ss) {
@@ -1031,7 +1030,7 @@ do_btrfs_subvolume_show (const char *subvolume)
            return NULL;
        }

-      p = analyze_line(p, &key, &value);
+      p = analyze_line(p, &key, &value, ':');
      }
    }

@@ -2083,3 +2082,4 @@ do_btrfs_image (char *const *sources, const char *image,

    return 0;
  }
+


The actual changes LGTM.

Thanks,


--
Yours Sincerely,

Cao Jin

_______________________________________________
Libguestfs mailing list
Libguestfs@redhat.com
https://www.redhat.com/mailman/listinfo/libguestfs