| Message ID | 20161114183644.15326-1-dsafonov@virtuozzo.com |
|---|---|
| State | Superseded |
| Series | "pstree: rename lookup_create_{item <=> pid}" |
| Headers | show |
diff --git a/criu/files-reg.c b/criu/files-reg.c index 1c3d32e0cbf7..047be3723857 100644 --- a/criu/files-reg.c +++ b/criu/files-reg.c @@ -361,7 +361,7 @@ static int open_remap_dead_process(struct reg_file_info *rfi, { struct pstree_item *helper; - helper = lookup_create_item(rfe->remap_id); + helper = lookup_create_pid(rfe->remap_id); if (!helper) return -1; diff --git a/criu/include/pstree.h b/criu/include/pstree.h index 5768b645021c..349ef4830061 100644 --- a/criu/include/pstree.h +++ b/criu/include/pstree.h @@ -74,7 +74,7 @@ extern struct pstree_item *__alloc_pstree_item(bool rst); #define alloc_pstree_item() __alloc_pstree_item(false) extern void init_pstree_helper(struct pstree_item *ret); -extern struct pstree_item *lookup_create_item(pid_t pid); +extern struct pstree_item *lookup_create_pid(pid_t pid); extern void pstree_insert_pid(pid_t pid, struct pid *pid_node); extern struct pid *pstree_pid_by_virt(pid_t pid); diff --git a/criu/pstree.c b/criu/pstree.c index 943518d98fea..081d4d000e1c 100644 --- a/criu/pstree.c +++ b/criu/pstree.c @@ -366,9 +366,9 @@ static int prepare_pstree_for_shell_job(void) pi->sid = current_sid; } - if (lookup_create_item(current_sid) == NULL) + if (lookup_create_pid(current_sid) == NULL) return -1; - if (lookup_create_item(current_gid) == NULL) + if (lookup_create_pid(current_gid) == NULL) return -1; return 0; @@ -379,7 +379,7 @@ static int prepare_pstree_for_shell_job(void) * it is not there yet. If pid_node isn't set, pstree_item * is inserted. */ -static struct pid *lookup_create_pid(pid_t pid, struct pid *pid_node) +static struct pid *lookup_create_item(pid_t pid, struct pid *pid_node) { struct rb_node *node = pid_root_rb.rb_node; struct rb_node **new = &pid_root_rb.rb_node; @@ -415,16 +415,16 @@ void pstree_insert_pid(pid_t pid, struct pid *pid_node) { struct pid* n; - n = lookup_create_pid(pid, pid_node); + n = lookup_create_item(pid, pid_node); BUG_ON(n != pid_node); } -struct pstree_item *lookup_create_item(pid_t pid) +struct pstree_item *lookup_create_pid(pid_t pid) { struct pid *node;; - node = lookup_create_pid(pid, NULL); + node = lookup_create_item(pid, NULL); if (!node) return NULL; BUG_ON(node->state == TASK_THREAD); @@ -492,7 +492,7 @@ static int read_pstree_image(pid_t *pid_max) break; ret = -1; - pi = lookup_create_item(e->pid); + pi = lookup_create_pid(e->pid); if (pi == NULL) break; BUG_ON(pi->pid.state != TASK_UNDEF); @@ -503,9 +503,9 @@ static int read_pstree_image(pid_t *pid_max) * be initialized when we meet PstreeEntry with this pid or * we will create helpers for them. */ - if (lookup_create_item(e->pgid) == NULL) + if (lookup_create_pid(e->pgid) == NULL) break; - if (lookup_create_item(e->sid) == NULL) + if (lookup_create_pid(e->sid) == NULL) break; pi->pid.virt = e->pid; @@ -556,7 +556,7 @@ static int read_pstree_image(pid_t *pid_max) pi->threads[i].state = TASK_THREAD; if (i == 0) continue; /* A thread leader is in a tree already */ - node = lookup_create_pid(pi->threads[i].virt, &pi->threads[i]); + node = lookup_create_item(pi->threads[i].virt, &pi->threads[i]); BUG_ON(node == NULL); if (node != &pi->threads[i]) { @@ -639,7 +639,7 @@ static int prepare_pstree_ids(void) pid = get_free_pid(); if (pid < 0) break; - helper = lookup_create_item(pid); + helper = lookup_create_pid(pid); if (helper == NULL) return -1;
On 11/14/2016 09:36 PM, Dmitry Safonov wrote: > I suggest to rename lookup_create_pid into lookup_create_item and > backwards. I think, the naming is misleading, now *_pid() looks for/creates > pstree item, which can be thread, undef task or anything else, But at the same time lookup_create_pid returns struct pid *, lookup_create_item returns struct pstree_item and you suggest to mess it up :) > while *_item() can't create/locate threads (because of BUG_ON()). > Let's swap them: > - lookup_create_item() will return pstree item - anything, process or thread > - lookup_create_pid() will only lookup/create processes, bugging on threads. > > Cc: Andrey Vagin <avagin@virtuozzo.com> > Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> > --- > criu/files-reg.c | 2 +- > criu/include/pstree.h | 2 +- > criu/pstree.c | 22 +++++++++++----------- > 3 files changed, 13 insertions(+), 13 deletions(-) > > diff --git a/criu/files-reg.c b/criu/files-reg.c > index 1c3d32e0cbf7..047be3723857 100644 > --- a/criu/files-reg.c > +++ b/criu/files-reg.c > @@ -361,7 +361,7 @@ static int open_remap_dead_process(struct reg_file_info *rfi, > { > struct pstree_item *helper; > > - helper = lookup_create_item(rfe->remap_id); > + helper = lookup_create_pid(rfe->remap_id); > if (!helper) > return -1; > > diff --git a/criu/include/pstree.h b/criu/include/pstree.h > index 5768b645021c..349ef4830061 100644 > --- a/criu/include/pstree.h > +++ b/criu/include/pstree.h > @@ -74,7 +74,7 @@ extern struct pstree_item *__alloc_pstree_item(bool rst); > #define alloc_pstree_item() __alloc_pstree_item(false) > extern void init_pstree_helper(struct pstree_item *ret); > > -extern struct pstree_item *lookup_create_item(pid_t pid); > +extern struct pstree_item *lookup_create_pid(pid_t pid); > extern void pstree_insert_pid(pid_t pid, struct pid *pid_node); > extern struct pid *pstree_pid_by_virt(pid_t pid); > > diff --git a/criu/pstree.c b/criu/pstree.c > index 943518d98fea..081d4d000e1c 100644 > --- a/criu/pstree.c > +++ b/criu/pstree.c > @@ -366,9 +366,9 @@ static int prepare_pstree_for_shell_job(void) > pi->sid = current_sid; > } > > - if (lookup_create_item(current_sid) == NULL) > + if (lookup_create_pid(current_sid) == NULL) > return -1; > - if (lookup_create_item(current_gid) == NULL) > + if (lookup_create_pid(current_gid) == NULL) > return -1; > > return 0; > @@ -379,7 +379,7 @@ static int prepare_pstree_for_shell_job(void) > * it is not there yet. If pid_node isn't set, pstree_item > * is inserted. > */ > -static struct pid *lookup_create_pid(pid_t pid, struct pid *pid_node) > +static struct pid *lookup_create_item(pid_t pid, struct pid *pid_node) > { > struct rb_node *node = pid_root_rb.rb_node; > struct rb_node **new = &pid_root_rb.rb_node; > @@ -415,16 +415,16 @@ void pstree_insert_pid(pid_t pid, struct pid *pid_node) > { > struct pid* n; > > - n = lookup_create_pid(pid, pid_node); > + n = lookup_create_item(pid, pid_node); > > BUG_ON(n != pid_node); > } > > -struct pstree_item *lookup_create_item(pid_t pid) > +struct pstree_item *lookup_create_pid(pid_t pid) > { > struct pid *node;; > > - node = lookup_create_pid(pid, NULL); > + node = lookup_create_item(pid, NULL); > if (!node) > return NULL; > BUG_ON(node->state == TASK_THREAD); > @@ -492,7 +492,7 @@ static int read_pstree_image(pid_t *pid_max) > break; > > ret = -1; > - pi = lookup_create_item(e->pid); > + pi = lookup_create_pid(e->pid); > if (pi == NULL) > break; > BUG_ON(pi->pid.state != TASK_UNDEF); > @@ -503,9 +503,9 @@ static int read_pstree_image(pid_t *pid_max) > * be initialized when we meet PstreeEntry with this pid or > * we will create helpers for them. > */ > - if (lookup_create_item(e->pgid) == NULL) > + if (lookup_create_pid(e->pgid) == NULL) > break; > - if (lookup_create_item(e->sid) == NULL) > + if (lookup_create_pid(e->sid) == NULL) > break; > > pi->pid.virt = e->pid; > @@ -556,7 +556,7 @@ static int read_pstree_image(pid_t *pid_max) > pi->threads[i].state = TASK_THREAD; > if (i == 0) > continue; /* A thread leader is in a tree already */ > - node = lookup_create_pid(pi->threads[i].virt, &pi->threads[i]); > + node = lookup_create_item(pi->threads[i].virt, &pi->threads[i]); > > BUG_ON(node == NULL); > if (node != &pi->threads[i]) { > @@ -639,7 +639,7 @@ static int prepare_pstree_ids(void) > pid = get_free_pid(); > if (pid < 0) > break; > - helper = lookup_create_item(pid); > + helper = lookup_create_pid(pid); > if (helper == NULL) > return -1; > >
On 11/17/2016 11:27 AM, Pavel Emelyanov wrote: > On 11/14/2016 09:36 PM, Dmitry Safonov wrote: >> I suggest to rename lookup_create_pid into lookup_create_item and >> backwards. I think, the naming is misleading, now *_pid() looks for/creates >> pstree item, which can be thread, undef task or anything else, > > But at the same time lookup_create_pid returns struct pid *, lookup_create_item > returns struct pstree_item and you suggest to mess it up :) Oh, yes - didn't catch that. Maybe something like lookup_create_process()? Or maybe leave as is - I just find it a little confusing that sounding "more generic" _create_item() doesn't create threads %) ...while _create_pid() does.
On 11/17/2016 12:35 PM, Dmitry Safonov wrote: > On 11/17/2016 11:27 AM, Pavel Emelyanov wrote: >> On 11/14/2016 09:36 PM, Dmitry Safonov wrote: >>> I suggest to rename lookup_create_pid into lookup_create_item and >>> backwards. I think, the naming is misleading, now *_pid() looks for/creates >>> pstree item, which can be thread, undef task or anything else, >> >> But at the same time lookup_create_pid returns struct pid *, lookup_create_item >> returns struct pstree_item and you suggest to mess it up :) > > Oh, yes - didn't catch that. > Maybe something like lookup_create_process()? > Or maybe leave as is - I just find it a little confusing that sounding > "more generic" _create_item() doesn't create threads %) > ...while _create_pid() does. Yup, some better names are welcome. Maybe lookup_create_pstree_item? This is exactly what it, you know, looks up and, well, creates. -- Pavel
I suggest to rename lookup_create_pid into lookup_create_item and backwards. I think, the naming is misleading, now *_pid() looks for/creates pstree item, which can be thread, undef task or anything else, while *_item() can't create/locate threads (because of BUG_ON()). Let's swap them: - lookup_create_item() will return pstree item - anything, process or thread - lookup_create_pid() will only lookup/create processes, bugging on threads. Cc: Andrey Vagin <avagin@virtuozzo.com> Signed-off-by: Dmitry Safonov <dsafonov@virtuozzo.com> --- criu/files-reg.c | 2 +- criu/include/pstree.h | 2 +- criu/pstree.c | 22 +++++++++++----------- 3 files changed, 13 insertions(+), 13 deletions(-)