Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
On 09.07.18 23:34, Junio C Hamano wrote:
> Beat Bolli writes:
>
>> The marco GIT_PATH_FUNC expands to a complete statement including the
>> semicolon. Remove two extra trailing semicolons.
>
> Wait a bit. The observation in the log message and the
> implementation of GIT_PATH_FUNC() do not match.
>
> #define GIT_PATH_FUNC(func, filename) \
> const char *func(void) \
> { \
> static char *ret; \
> if (!ret) \
> ret = git_pathdup(filename); \
> return ret; \
> }
>
> The code generated does "include semicolon" but that is not why the
> caller should place semicolon after the closing parens. Perhaps
> replace "including the semicolon." with something else, like ", and
> adding a semicolon after it not only is unnecessary but is wrong."
> or soemthing like that?
This message is fixed in the non-RFC series that I sent at 19:25 UTC. I
noticed the error after the message from Philip Oakley.
Beat
Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
Beat Bolli writes:
> The marco GIT_PATH_FUNC expands to a complete statement including the
> semicolon. Remove two extra trailing semicolons.
Wait a bit. The observation in the log message and the
implementation of GIT_PATH_FUNC() do not match.
#define GIT_PATH_FUNC(func, filename) \
const char *func(void) \
{ \
static char *ret; \
if (!ret) \
ret = git_pathdup(filename); \
return ret; \
}
The code generated does "include semicolon" but that is not why the
caller should place semicolon after the closing parens. Perhaps
replace "including the semicolon." with something else, like ", and
adding a semicolon after it not only is unnecessary but is wrong."
or soemthing like that?
It is a bit unfortunate that we need to live with a slight uglyness
of the resulting source code, unlike e.g. define_commit_slab() that
can (and must) end with a semicolon, which gives us a more natural
look. But that is a separate issue.
>
> Signed-off-by: Beat Bolli
> ---
> sequencer.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/sequencer.c b/sequencer.c
> index 5354d4d51e..66e7073995 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done,
> "rebase-merge/done")
> * The file to keep track of how many commands were already processed (e.g.
> * for the prompt).
> */
> -static GIT_PATH_FUNC(rebase_path_msgnum, "rebase-merge/msgnum");
> +static GIT_PATH_FUNC(rebase_path_msgnum, "rebase-merge/msgnum")
> /*
> * The file to keep track of how many commands are to be processed in total
> * (e.g. for the prompt).
> */
> -static GIT_PATH_FUNC(rebase_path_msgtotal, "rebase-merge/end");
> +static GIT_PATH_FUNC(rebase_path_msgtotal, "rebase-merge/end")
> /*
> * The commit message that is planned to be used for any changes that
> * need to be committed following a user interaction.
Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
Am 08.07.2018 23:17, schrieb Philip Oakley: From: "Eric Sunshine" To: "Beat Bolli" On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli wrote: The marco GIT_PATH_FUNC expands to a complete statement including the s/marco/macro/ ACK. In addition, the whole sentence is wrong: GIT_PATH_FUNC defines a function, not a statement. Will be fixes in a reroll. semicolon. Remove two extra trailing semicolons. Signed-off-by: Beat Bolli --- sequencer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) While you're at it, perhaps it would be a good idea to fix the example in path.h which teaches the "wrong" way: /* * You can define a static memoized git path like: * *static GIT_PATH_FUNC(git_path_foo, "FOO"); * * or use one of the global ones below. */ Thanks, Beat
Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
From: "Eric Sunshine" To: "Beat Bolli" On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli wrote: The marco GIT_PATH_FUNC expands to a complete statement including the s/marco/macro/ semicolon. Remove two extra trailing semicolons. Signed-off-by: Beat Bolli --- sequencer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) While you're at it, perhaps it would be a good idea to fix the example in path.h which teaches the "wrong" way: /* * You can define a static memoized git path like: * *static GIT_PATH_FUNC(git_path_foo, "FOO"); * * or use one of the global ones below. */
Re: [RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
On Sun, Jul 8, 2018 at 10:44 AM Beat Bolli wrote: > The marco GIT_PATH_FUNC expands to a complete statement including the > semicolon. Remove two extra trailing semicolons. > > Signed-off-by: Beat Bolli > --- > sequencer.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) While you're at it, perhaps it would be a good idea to fix the example in path.h which teaches the "wrong" way: /* * You can define a static memoized git path like: * *static GIT_PATH_FUNC(git_path_foo, "FOO"); * * or use one of the global ones below. */
[RFC PATCH 4/6] sequencer.c: avoid empty statements at top level
The marco GIT_PATH_FUNC expands to a complete statement including the semicolon. Remove two extra trailing semicolons. Signed-off-by: Beat Bolli --- sequencer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 5354d4d51e..66e7073995 100644 --- a/sequencer.c +++ b/sequencer.c @@ -62,12 +62,12 @@ static GIT_PATH_FUNC(rebase_path_done, "rebase-merge/done") * The file to keep track of how many commands were already processed (e.g. * for the prompt). */ -static GIT_PATH_FUNC(rebase_path_msgnum, "rebase-merge/msgnum"); +static GIT_PATH_FUNC(rebase_path_msgnum, "rebase-merge/msgnum") /* * The file to keep track of how many commands are to be processed in total * (e.g. for the prompt). */ -static GIT_PATH_FUNC(rebase_path_msgtotal, "rebase-merge/end"); +static GIT_PATH_FUNC(rebase_path_msgtotal, "rebase-merge/end") /* * The commit message that is planned to be used for any changes that * need to be committed following a user interaction. -- 2.15.0.rc1.299.gda03b47c3
