[PATCH 2/9] unwind: give the field in tcb used for unwinding longer name

[Dmitry V. Levin]

On Wed, Mar 14, 2018 at 02:28:07AM +0900, Masatake YAMATO wrote:
> unwind: give the field in tcb used for unwinding longer name

Sorry but that's not a sentence.  This is a sentence:
"unwind: give a longer name to the field in struct tcb that is used for unwinding"
but it's too long.  What about a shorter alternative, e.g.

unwind: add unwind_ prefix to struct tcb.queue field and its type

> tcb has field named `queue' for unwinding.  It is too simple name and
> can conflicts with fields introduced in the future.

s/tcb/struct tcb/
s/has field/has a field/
s/It is too simple name/The name is too common/
s/can conflicts/can conflict/ or can cause conflicts
s/fields introduced in the future/fields that might be introduced in the future/

The field is there for unwinding, but its name "queue" is not for unwinding.
How do you like this edition:

The names given to struct tcb.queue field and its type are too common,
this may cause conflicts in the future if new fields are added
to struct tcb. 

> This change gives `unwind_' as prefix to the field.

s/gives/adds/
s/as prefix/prefix/
s/to the field/to the field and its type/

I suppose we can do without this sentence.

> * defs.h (struct tcb): Rename a field, `queue' to `unwind_queue'.

Rename queue field to unwind_queue.

> Rename its type, `queue_t' to `unwind_queue_t'.

Rename its type queue_t to unwind_queue_t.

> * unwind.c (struct unwind_queue_t): Rename `queue_t'.

(struct queue_t): Rename to unwind_queue_t.

> (queue_print): Reflect the above field and type renaming.
> (unwind_tcb_init, unwind_tcb_fin, queue_put, queue_print,
> unwind_print_stacktrace, and unwind_capture_stacktrace): Ditto.

All users updated.

> Signed-off-by: Masatake YAMATO <yamato at redhat.com>
> ---
>  defs.h   |  2 +-
>  unwind.c | 40 +++++++++++++++++++++++-----------------
>  2 files changed, 24 insertions(+), 18 deletions(-)
> 
> diff --git a/defs.h b/defs.h
> index 4d78e0d0..f8867157 100644
> --- a/defs.h
> +++ b/defs.h
> @@ -224,7 +224,7 @@ struct tcb {
>  
>  #ifdef USE_LIBUNWIND
>  	void *unwind_ctx;
> -	struct queue_t *queue;
> +	struct unwind_queue_t *unwind_queue;
>  #endif
>  };
>  
> diff --git a/unwind.c b/unwind.c
> index 1b06aa3d..14a656a4 100644
> --- a/unwind.c
> +++ b/unwind.c
> @@ -57,12 +57,12 @@ struct call_t {
>  	char *output_line;
>  };
>  
> -struct queue_t {
> +struct unwind_queue_t {
>  	struct call_t *tail;
>  	struct call_t *head;
>  };
>  
> -static void queue_print(struct queue_t *queue);
> +static void queue_print(struct unwind_queue_t *queue);
>  
>  static unw_addr_space_t libunwind_as;
>  
> @@ -88,17 +88,17 @@ unwind_tcb_init(struct tcb *tcp)
>  	if (!tcp->unwind_ctx)
>  		perror_msg_and_die("_UPT_create");
>  
> -	tcp->queue = xmalloc(sizeof(*tcp->queue));
> -	tcp->queue->head = NULL;
> -	tcp->queue->tail = NULL;
> +	tcp->unwind_queue = xmalloc(sizeof(*tcp->unwind_queue));
> +	tcp->unwind_queue->head = NULL;
> +	tcp->unwind_queue->tail = NULL;
>  }
>  
>  void
>  unwind_tcb_fin(struct tcb *tcp)
>  {
> -	queue_print(tcp->queue);
> -	free(tcp->queue);
> -	tcp->queue = NULL;
> +	queue_print(tcp->unwind_queue);
> +	free(tcp->unwind_queue);
> +	tcp->unwind_queue = NULL;
>  
>  	_UPT_destroy((struct UPT_info *)tcp->unwind_ctx);
>  	tcp->unwind_ctx = NULL;
> @@ -308,7 +308,7 @@ sprint_call_or_error(const char *binary_filename,
>   * queue manipulators
>   */
>  static void
> -queue_put(struct queue_t *queue,
> +queue_put(struct unwind_queue_t *queue,
>  	  const char *binary_filename,
>  	  const char *symbol_name,
>  	  unw_word_t function_offset,
> @@ -358,7 +358,7 @@ queue_put_error(void *queue,
>  }
>  
>  static void
> -queue_print(struct queue_t *queue)
> +queue_print(struct unwind_queue_t *queue)
>  {
>  	struct call_t *call, *tmp;
>  
> @@ -387,21 +387,24 @@ queue_print(struct queue_t *queue)
>  void
>  unwind_print_stacktrace(struct tcb *tcp)
>  {
> +	struct unwind_queue_t *queue;
> +

This doesn't look like a simple rename of tcp->queue.
Why do you need a proxy variable?


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20180315/9237774f/attachment.bin>