Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Cyrill Gorcunov <gorcunov@gmail.com>
Cc: tml <tarantool-patches@dev.tarantool.org>
Subject: Re: [Tarantool-patches] [PATCH v12 3/8] module_cache: improve naming
Date: Sat, 30 Jan 2021 19:53:03 +0100	[thread overview]
Message-ID: <d5d343d3-0c85-c89d-46cc-2cf003145c9e@tarantool.org> (raw)
In-Reply-To: <20210124223256.GD2174@grain>

Hi!

On 24.01.2021 23:32, Cyrill Gorcunov wrote:
> On Sun, Jan 24, 2021 at 05:27:12PM +0100, Vladislav Shpilevoy wrote:
>> Thanks for the patch!
>>
>> Very dubious commit. To me the old and new names look the same.
>> 'func_name' was even better than 'func_name_desc'. At least
>> because it was shorter, and 'desc' does not add any more meaning.
>>
>> On 18.01.2021 21:35, Cyrill Gorcunov via Tarantool-patches wrote:
>>>  - rename func_name to func_name_desc because
>>>    it is not just a name but rather a name descriptor
>>>    which includes symbol address
>>
>> I don't see any address in it. Only name tokens.
> 
> struct func_name_desc {
> 	/**
> 	 * Null-terminated symbol name, e.g.
> 	 * "func" for "mod.submod.func".
> 	 */
> --->	const char *sym;
> 	/**
> 	 * Package name, e.g. "mod.submod" for
> 	 * "mod.submod.func".
> 	 */
> 	const char *package;
> 	/**
> 	 * A pointer to the last character in ->package + 1.
> 	 */
> 	const char *package_end;
> };
> 
> I marked the address.

You said "symbol address". But it is not a symbol address. It is
an address pointing at a part of the name string. Not at the
actual symbol, which you need to load later.

> The structure is not a function name
> but consists of two parts - package path and function name
> itself and maybe something new will be added in future.

In this case 'name desc' is also invalid - it does not describe
the name. Because there is also a path which is not a part of
the name.

It would be more correct to call it func_path then.

> So func_name is suitable for plain names but not for strings
> where some part of it has a special meaning with hidden
> extension inside, hereby a descriptor.

In this case you didn't really change it - it was about function
name and stayed about function name. It should be func_path then.
If you really want to rename.

> And don't forget
> about grepability, without this name change grep returns
> a number of irrelevant results.

This struct is defined and used in a single file. It is not that
hard to find it here. Especially if you use some smarter tools.
For instance, I use full text search in Sublime, and have never had
any issues with finding anything in Tarantool sources. Plus for
structs and functions you usually want to use ctags (Sublime also
generates these tags). Maybe it is only hard when you use command
line and bare grep.

  reply	other threads:[~2021-01-30 18:53 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-18 20:35 [Tarantool-patches] [PATCH v12 0/8] box: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 1/8] box/func: factor out c function entry structure Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 2/8] module_cache: move module handling into own subsystem Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:26   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-25  8:52     ` Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 3/8] module_cache: improve naming Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:27   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-24 22:32     ` Cyrill Gorcunov via Tarantool-patches
2021-01-30 18:53       ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-02-01 11:41         ` Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 4/8] module_cache: direct update a cache value on reload Cyrill Gorcunov via Tarantool-patches
2021-01-19 12:46   ` Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:27     ` Vladislav Shpilevoy via Tarantool-patches
2021-01-24 22:26       ` Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 5/8] module_cache: rename calls to ref in module structure Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:27   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-25 10:29     ` Cyrill Gorcunov via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 6/8] module_cache: provide helpers to load and unload modules Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:28   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 7/8] box/cmod: implement cmod Lua module Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:28   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-25 16:50     ` Cyrill Gorcunov via Tarantool-patches
2021-01-30 18:53       ` Vladislav Shpilevoy via Tarantool-patches
2021-01-18 20:35 ` [Tarantool-patches] [PATCH v12 8/8] test: box/cfunc -- add cmod test Cyrill Gorcunov via Tarantool-patches
2021-01-24 16:28   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-24 16:26 ` [Tarantool-patches] [PATCH v12 0/8] box: implement cmod Lua module Vladislav Shpilevoy via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=d5d343d3-0c85-c89d-46cc-2cf003145c9e@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v12 3/8] module_cache: improve naming' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox