| Message ID | 20221213160045.28170-9-jgross@suse.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/xenstore: do some cleanup and fixes | expand |
Hi Juergen, On 13/12/2022 16:00, Juergen Gross wrote: > @@ -313,19 +302,19 @@ const char *dump_state_watches(FILE *fp, struct connection *conn, > unsigned int conn_id) > { > const char *ret = NULL; > + const char *watch_path; > struct watch *watch; > struct xs_state_watch sw; > struct xs_state_record_header head; > - const char *path; > > head.type = XS_STATE_TYPE_WATCH; > > list_for_each_entry(watch, &conn->watches, list) { > head.length = sizeof(sw); > > + watch_path = get_watch_path(watch, watch->node); It is not clear to me why you call get_watch_path() earlier and also rename the variable. I don't mind the new name, but it doesn't feel like it belongs to this patch as the code in duymp_state_watches() would not be changed otherwise. Cheers, > sw.conn_id = conn_id; > - path = get_watch_path(watch, watch->node); > - sw.path_length = strlen(path) + 1; > + sw.path_length = strlen(watch_path) + 1; > sw.token_length = strlen(watch->token) + 1; > head.length += sw.path_length + sw.token_length; > head.length = ROUNDUP(head.length, 3); > @@ -334,7 +323,7 @@ const char *dump_state_watches(FILE *fp, struct connection *conn, > if (fwrite(&sw, sizeof(sw), 1, fp) != 1) > return "Dump watch state error"; > > - if (fwrite(path, sw.path_length, 1, fp) != 1) > + if (fwrite(watch_path, sw.path_length, 1, fp) != 1) > return "Dump watch path error"; > if (fwrite(watch->token, sw.token_length, 1, fp) != 1) > return "Dump watch token error"; Cheers,
On 20.12.22 20:42, Julien Grall wrote: > Hi Juergen, > > On 13/12/2022 16:00, Juergen Gross wrote: >> @@ -313,19 +302,19 @@ const char *dump_state_watches(FILE *fp, struct >> connection *conn, >> unsigned int conn_id) >> { >> const char *ret = NULL; >> + const char *watch_path; >> struct watch *watch; >> struct xs_state_watch sw; >> struct xs_state_record_header head; >> - const char *path; >> head.type = XS_STATE_TYPE_WATCH; >> list_for_each_entry(watch, &conn->watches, list) { >> head.length = sizeof(sw); >> + watch_path = get_watch_path(watch, watch->node); > > It is not clear to me why you call get_watch_path() earlier and also rename the > variable. > > I don't mind the new name, but it doesn't feel like it belongs to this patch as > the code in duymp_state_watches() would not be changed otherwise. Both changes are the result of V1 of the series. Will undo them. Juergen
diff --git a/tools/xenstore/xenstored_watch.c b/tools/xenstore/xenstored_watch.c index 75748ac109..5c0f764781 100644 --- a/tools/xenstore/xenstored_watch.c +++ b/tools/xenstore/xenstored_watch.c @@ -39,8 +39,8 @@ struct watch /* Current outstanding events applying to this watch. */ struct list_head events; - /* Is this relative to connnection's implicit path? */ - const char *relative_path; + /* Offset into path for skipping prefix (used for relative paths). */ + unsigned int prefix_len; char *token; char *node; @@ -66,15 +66,7 @@ static bool is_child(const char *child, const char *parent) static const char *get_watch_path(const struct watch *watch, const char *name) { - const char *path = name; - - if (watch->relative_path) { - path += strlen(watch->relative_path); - if (*path == '/') /* Could be "" */ - path++; - } - - return path; + return name + watch->prefix_len; } /* @@ -211,10 +203,7 @@ static struct watch *add_watch(struct connection *conn, char *path, char *token, no_quota_check)) goto nomem; - if (relative) - watch->relative_path = get_implicit_path(conn); - else - watch->relative_path = NULL; + watch->prefix_len = relative ? strlen(get_implicit_path(conn)) + 1 : 0; INIT_LIST_HEAD(&watch->events); @@ -313,19 +302,19 @@ const char *dump_state_watches(FILE *fp, struct connection *conn, unsigned int conn_id) { const char *ret = NULL; + const char *watch_path; struct watch *watch; struct xs_state_watch sw; struct xs_state_record_header head; - const char *path; head.type = XS_STATE_TYPE_WATCH; list_for_each_entry(watch, &conn->watches, list) { head.length = sizeof(sw); + watch_path = get_watch_path(watch, watch->node); sw.conn_id = conn_id; - path = get_watch_path(watch, watch->node); - sw.path_length = strlen(path) + 1; + sw.path_length = strlen(watch_path) + 1; sw.token_length = strlen(watch->token) + 1; head.length += sw.path_length + sw.token_length; head.length = ROUNDUP(head.length, 3); @@ -334,7 +323,7 @@ const char *dump_state_watches(FILE *fp, struct connection *conn, if (fwrite(&sw, sizeof(sw), 1, fp) != 1) return "Dump watch state error"; - if (fwrite(path, sw.path_length, 1, fp) != 1) + if (fwrite(watch_path, sw.path_length, 1, fp) != 1) return "Dump watch path error"; if (fwrite(watch->token, sw.token_length, 1, fp) != 1) return "Dump watch token error";
Instead of storing a pointer to the path which is prepended to relative paths in struct watch, just use the length of the prepended path. It should be noted that the now removed special case of the relative path being "" in get_watch_path() can't happen at all. Signed-off-by: Juergen Gross <jgross@suse.com> --- V2: - don't open code get_watch_path() (Julien Grall) --- tools/xenstore/xenstored_watch.c | 27 ++++++++------------------- 1 file changed, 8 insertions(+), 19 deletions(-)