Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7
@ 2021-03-04 14:25 Cyrill Gorcunov via Tarantool-patches
  2021-03-05  8:16 ` Aleksandr Lyapunov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-04 14:25 UTC (permalink / raw)
  To: tml; +Cc: Alexander Turenko

The order of evaluation of arguments passed to a function
is not a part of language standart but compiler specifics.
Thus make sure we pass proper "errno" to xdir_say_gc.

Note that this is not critical issue: in case of hit
we simply get wrong syserror message when file not
present on a storage device.

Reported-by: Aleksandr Lyapunov <alyapunov@tarantool.org>
Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
Guys, I didn't create a branch for this fix since it is not
critical and there is no issue for it. Even if the problem
occurs it won't affect the workflow so it is rather a cosmetic
fix.

 src/box/xlog.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/box/xlog.c b/src/box/xlog.c
index 974f460be..462245d27 100644
--- a/src/box/xlog.c
+++ b/src/box/xlog.c
@@ -684,10 +684,13 @@ xdir_collect_garbage(struct xdir *dir, int64_t signature, unsigned flags)
 	       vclock_sum(vclock) < signature) {
 		const char *filename =
 			xdir_format_filename(dir, vclock_sum(vclock), NONE);
-		if (flags & XDIR_GC_ASYNC)
+		if (flags & XDIR_GC_ASYNC) {
 			eio_unlink(filename, 0, xdir_complete_gc, NULL);
-		else
-			xdir_say_gc(unlink(filename), errno, filename);
+		} else {
+			int rc = unlink(filename);
+			int _errno = rc != 0 ? errno : 0;
+			xdir_say_gc(rc, _errno, filename);
+		}
 		vclockset_remove(&dir->index, vclock);
 		free(vclock);
 
-- 
2.29.2


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7
  2021-03-04 14:25 [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7 Cyrill Gorcunov via Tarantool-patches
@ 2021-03-05  8:16 ` Aleksandr Lyapunov via Tarantool-patches
  2021-03-05  8:26   ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Aleksandr Lyapunov via Tarantool-patches @ 2021-03-05  8:16 UTC (permalink / raw)
  To: Cyrill Gorcunov, tml; +Cc: Alexander Turenko

Hi! thanks for the patch!

Actually we found the error an fixed it in #5823.
You can see identical commit 733081f7042974f8fee9431e46cd45d2f1601438 .
But again thanks for your cooperation!

On 04.03.2021 17:25, Cyrill Gorcunov wrote:
> The order of evaluation of arguments passed to a function
> is not a part of language standart but compiler specifics.
> Thus make sure we pass proper "errno" to xdir_say_gc.
>
> Note that this is not critical issue: in case of hit
> we simply get wrong syserror message when file not
> present on a storage device.
>
> Reported-by: Aleksandr Lyapunov <alyapunov@tarantool.org>
> Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
> ---
> Guys, I didn't create a branch for this fix since it is not
> critical and there is no issue for it. Even if the problem
> occurs it won't affect the workflow so it is rather a cosmetic
> fix.
>
>   src/box/xlog.c | 9 ++++++---
>   1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/src/box/xlog.c b/src/box/xlog.c
> index 974f460be..462245d27 100644
> --- a/src/box/xlog.c
> +++ b/src/box/xlog.c
> @@ -684,10 +684,13 @@ xdir_collect_garbage(struct xdir *dir, int64_t signature, unsigned flags)
>   	       vclock_sum(vclock) < signature) {
>   		const char *filename =
>   			xdir_format_filename(dir, vclock_sum(vclock), NONE);
> -		if (flags & XDIR_GC_ASYNC)
> +		if (flags & XDIR_GC_ASYNC) {
>   			eio_unlink(filename, 0, xdir_complete_gc, NULL);
> -		else
> -			xdir_say_gc(unlink(filename), errno, filename);
> +		} else {
> +			int rc = unlink(filename);
> +			int _errno = rc != 0 ? errno : 0;
> +			xdir_say_gc(rc, _errno, filename);
> +		}
>   		vclockset_remove(&dir->index, vclock);
>   		free(vclock);
>   

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7
  2021-03-05  8:16 ` Aleksandr Lyapunov via Tarantool-patches
@ 2021-03-05  8:26   ` Cyrill Gorcunov via Tarantool-patches
  2021-03-05 10:14     ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-05  8:26 UTC (permalink / raw)
  To: Aleksandr Lyapunov; +Cc: tml, Alexander Turenko

On Fri, Mar 05, 2021 at 11:16:01AM +0300, Aleksandr Lyapunov wrote:
> Hi! thanks for the patch!
> 
> Actually we found the error an fixed it in #5823.
> You can see identical commit 733081f7042974f8fee9431e46cd45d2f1601438 .
> But again thanks for your cooperation!

Aha! I see, thanks! You know the patch you merged in actually still buggy
---
+               } else {
+                       int rc = unlink(filename);
+                       xdir_say_gc(rc, errno, filename);
+
}

The compiler has all rights to fetch @errno _before_ evaluating_ unlink,
there is no dependency. We might need a barrier. My patch did

+			int rc = unlink(filename);
+			int _errno = rc != 0 ? errno : 0;
+			xdir_say_gc(rc, _errno, filename);

here is _errno depends on unlink with a purpose.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7
  2021-03-05  8:26   ` Cyrill Gorcunov via Tarantool-patches
@ 2021-03-05 10:14     ` Cyrill Gorcunov via Tarantool-patches
  0 siblings, 0 replies; 4+ messages in thread
From: Cyrill Gorcunov via Tarantool-patches @ 2021-03-05 10:14 UTC (permalink / raw)
  To: Aleksandr Lyapunov; +Cc: tml, Alexander Turenko

On Fri, Mar 05, 2021 at 11:26:13AM +0300, Cyrill Gorcunov wrote:
> On Fri, Mar 05, 2021 at 11:16:01AM +0300, Aleksandr Lyapunov wrote:
> > Hi! thanks for the patch!
> > 
> > Actually we found the error an fixed it in #5823.
> > You can see identical commit 733081f7042974f8fee9431e46cd45d2f1601438 .
> > But again thanks for your cooperation!

After conversation indeed the commit above should fix the problem. Thanks!

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-03-05 10:14 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-04 14:25 [Tarantool-patches] [PATCH] xlog: fix arguments processing in commit 8e429f4b7 Cyrill Gorcunov via Tarantool-patches
2021-03-05  8:16 ` Aleksandr Lyapunov via Tarantool-patches
2021-03-05  8:26   ` Cyrill Gorcunov via Tarantool-patches
2021-03-05 10:14     ` Cyrill Gorcunov via Tarantool-patches

Tarantool development patches archive

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://lists.tarantool.org/tarantool-patches/0 tarantool-patches/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 tarantool-patches tarantool-patches/ https://lists.tarantool.org/tarantool-patches \
		tarantool-patches@dev.tarantool.org.
	public-inbox-index tarantool-patches

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git