[oe-core][PATCH 0/1] pseudo - renaming to self

[oe-core][PATCH 0/1] pseudo - renaming to self

[Joe Slater]

Bugzilla 13426

Simple test using rename() and renameat() attached to case.

Joe Slater (1):
  pseudo: fix renaming to self

 .../pseudo/files/rename.patch                 | 73 +++++++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
 2 files changed, 74 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch

-- 
2.17.1

[oe-core][PATCH 1/1] pseudo: fix renaming to self

[Joe Slater]

Pseudo tests for an item being renamed to itself only after
information about it has been deleted.  Move the test to before
we change the database.

Note that pseudo does not support renameat2(), but neither does
glibc.

Signed-off-by: Joe Slater <joe.slater@windriver.com>
---
 .../pseudo/files/rename.patch                 | 73 +++++++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
 2 files changed, 74 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch

diff --git a/meta/recipes-devtools/pseudo/files/rename.patch b/meta/recipes-devtools/pseudo/files/rename.patch
new file mode 100644
index 0000000000..bc344db3b5
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/rename.patch
@@ -0,0 +1,73 @@
+pseudo: fix renaming to self
+
+The pseudo rename guts test for an item being renamed to
+itself, only after information about it has been deleted.
+We move the test to before we play with the database.
+
+Note that pseudo does not support renameat2().
+
+Upstream-Status: Pending
+
+Signed-off-by: Joe Slater <joe.slater@windriver.com>
+
+
+--- a/ports/unix/guts/rename.c
++++ b/ports/unix/guts/rename.c
+@@ -29,6 +29,14 @@
+ 	newrc = base_lstat(newpath, &newbuf);
+ 	oldrc = base_lstat(oldpath, &oldbuf);
+ 
++	/* nothing to do for a "rename" of a link to itself */
++	if (newrc != -1 && oldrc != -1 &&
++	    newbuf.st_dev == oldbuf.st_dev &&
++	    newbuf.st_ino == oldbuf.st_ino) {
++		pseudo_debug(PDBGF_OP, "rename: paths are the same\n");
++		return real_rename(oldpath, newpath);
++        }
++
+ 	errno = save_errno;
+ 
+ 	/* newpath must be removed. */
+@@ -58,12 +66,6 @@
+ 		return rc;
+ 	}
+ 	save_errno = errno;
+-	/* nothing to do for a "rename" of a link to itself */
+-	if (newrc != -1 && oldrc != -1 &&
+-	    newbuf.st_dev == oldbuf.st_dev &&
+-	    newbuf.st_ino == oldbuf.st_ino) {
+-		return rc;
+-        }
+ 
+ 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
+ 	 * the corrected path yourself.  You can rename over a directory only
+--- a/ports/unix/guts/renameat.c
++++ b/ports/unix/guts/renameat.c
+@@ -41,6 +41,14 @@
+ 	newrc = base_fstatat(newdirfd, newpath, &newbuf, AT_SYMLINK_NOFOLLOW);
+ #endif
+ 
++	/* nothing to do for a "rename" of a link to itself */
++	if (newrc != -1 && oldrc != -1 &&
++	    newbuf.st_dev == oldbuf.st_dev &&
++	    newbuf.st_ino == oldbuf.st_ino) {
++		pseudo_debug(PDBGF_OP, "renameat: paths are the same\n");
++		return real_renameat(olddirfd, oldpath, newdirfd, newpath);
++        }
++
+ 	errno = save_errno;
+ 
+ 	/* newpath must be removed. */
+@@ -71,12 +79,6 @@
+ 		return rc;
+ 	}
+ 	save_errno = errno;
+-	/* nothing to do for a "rename" of a link to itself */
+-	if (newrc != -1 && oldrc != -1 &&
+-	    newbuf.st_dev == oldbuf.st_dev &&
+-	    newbuf.st_ino == oldbuf.st_ino) {
+-		return rc;
+-        }
+ 
+ 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
+ 	 * the corrected path yourself.  You can rename over a directory only
diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
index 9a22304bba..8d8cf8d523 100644
--- a/meta/recipes-devtools/pseudo/pseudo_git.bb
+++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
@@ -4,6 +4,7 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
            file://0001-configure-Prune-PIE-flags.patch \
            file://fallback-passwd \
            file://fallback-group \
+           file://rename.patch \
            "
 
 SRCREV = "8efb082863ff0ceec7b7e46f9a44750e12f48039"
-- 
2.17.1

Re: [oe-core][PATCH 1/1] pseudo: fix renaming to self

[Randy MacLeod]

Add Seebs.

Joe,
How did you test this change?

../Randy

On 2020-08-26 2:58 p.m., Joe Slater wrote:
> Pseudo tests for an item being renamed to itself only after
> information about it has been deleted.  Move the test to before
> we change the database.
>
> Note that pseudo does not support renameat2(), but neither does
> glibc.
>
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>   .../pseudo/files/rename.patch                 | 73 +++++++++++++++++++
>   meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
>   2 files changed, 74 insertions(+)
>   create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch
>
> diff --git a/meta/recipes-devtools/pseudo/files/rename.patch b/meta/recipes-devtools/pseudo/files/rename.patch
> new file mode 100644
> index 0000000000..bc344db3b5
> --- /dev/null
> +++ b/meta/recipes-devtools/pseudo/files/rename.patch
> @@ -0,0 +1,73 @@
> +pseudo: fix renaming to self
> +
> +The pseudo rename guts test for an item being renamed to
> +itself, only after information about it has been deleted.
> +We move the test to before we play with the database.
> +
> +Note that pseudo does not support renameat2().
> +
> +Upstream-Status: Pending
> +
> +Signed-off-by: Joe Slater <joe.slater@windriver.com>
> +
> +
> +--- a/ports/unix/guts/rename.c
> ++++ b/ports/unix/guts/rename.c
> +@@ -29,6 +29,14 @@
> + 	newrc = base_lstat(newpath, &newbuf);
> + 	oldrc = base_lstat(oldpath, &oldbuf);
> +
> ++	/* nothing to do for a "rename" of a link to itself */
> ++	if (newrc != -1 && oldrc != -1 &&
> ++	    newbuf.st_dev == oldbuf.st_dev &&
> ++	    newbuf.st_ino == oldbuf.st_ino) {
> ++		pseudo_debug(PDBGF_OP, "rename: paths are the same\n");
> ++		return real_rename(oldpath, newpath);
> ++        }
> ++
> + 	errno = save_errno;
> +
> + 	/* newpath must be removed. */
> +@@ -58,12 +66,6 @@
> + 		return rc;
> + 	}
> + 	save_errno = errno;
> +-	/* nothing to do for a "rename" of a link to itself */
> +-	if (newrc != -1 && oldrc != -1 &&
> +-	    newbuf.st_dev == oldbuf.st_dev &&
> +-	    newbuf.st_ino == oldbuf.st_ino) {
> +-		return rc;
> +-        }
> +
> + 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
> + 	 * the corrected path yourself.  You can rename over a directory only
> +--- a/ports/unix/guts/renameat.c
> ++++ b/ports/unix/guts/renameat.c
> +@@ -41,6 +41,14 @@
> + 	newrc = base_fstatat(newdirfd, newpath, &newbuf, AT_SYMLINK_NOFOLLOW);
> + #endif
> +
> ++	/* nothing to do for a "rename" of a link to itself */
> ++	if (newrc != -1 && oldrc != -1 &&
> ++	    newbuf.st_dev == oldbuf.st_dev &&
> ++	    newbuf.st_ino == oldbuf.st_ino) {
> ++		pseudo_debug(PDBGF_OP, "renameat: paths are the same\n");
> ++		return real_renameat(olddirfd, oldpath, newdirfd, newpath);
> ++        }
> ++
> + 	errno = save_errno;
> +
> + 	/* newpath must be removed. */
> +@@ -71,12 +79,6 @@
> + 		return rc;
> + 	}
> + 	save_errno = errno;
> +-	/* nothing to do for a "rename" of a link to itself */
> +-	if (newrc != -1 && oldrc != -1 &&
> +-	    newbuf.st_dev == oldbuf.st_dev &&
> +-	    newbuf.st_ino == oldbuf.st_ino) {
> +-		return rc;
> +-        }
> +
> + 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
> + 	 * the corrected path yourself.  You can rename over a directory only
> diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
> index 9a22304bba..8d8cf8d523 100644
> --- a/meta/recipes-devtools/pseudo/pseudo_git.bb
> +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
> @@ -4,6 +4,7 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
>              file://0001-configure-Prune-PIE-flags.patch \
>              file://fallback-passwd \
>              file://fallback-group \
> +           file://rename.patch \
>              "
>   
>   SRCREV = "8efb082863ff0ceec7b7e46f9a44750e12f48039"


-- 
# Randy MacLeod
# Wind River Linux

Re: [oe-core][PATCH 1/1] pseudo: fix renaming to self

[Randy MacLeod]

[-- Attachment #1: Type: text/plain, Size: 5676 bytes --]

On 2020-08-26 4:04 p.m., Randy MacLeod wrote:
>
> Add Seebs.
>
> Joe,
> How did you test this change?

Oops, I see that you attached the test to the defect:

https://bugzilla.yoctoproject.org/show_bug.cgi?id=13426

See below for (barfy) src. ;-)

../Randy


/*
  * test for pseudo renaming
  *
  *   Run in a directory named arf with files arf and arfat existing.
  */


#include <fcntl.h>
#include <stdio.h>

int main(int argc, char **argv)
  {
    if (!rename("arf","barf"))  printf("arf to barf\n");
    if (!rename("barf","barf"))  printf("barf to barf\n");
    if (!rename("barf","../arf/barf"))  printf("barf to ../arf/barf\n");

    if (!renameat(AT_FDCWD,"arfat",AT_FDCWD,"barfat"))
       printf("arfat to barfat\n");
    if (!renameat(AT_FDCWD,"barfat",AT_FDCWD,"barfat"))
       printf("barfat to barfat\n");

#ifdef UNDEFINED
    if (!renameat2(AT_FDCWD,"arf2",AT_FDCWD,"barf2",0))
       printf("arf2 to barf2\n");
    if (!renameat2(AT_FDCWD,"barf2",AT_FDCWD,"barf2",0))
       printf("barf2 to barf2\n");
#endif

    return 0;

  }

>
> ../Randy
>
> On 2020-08-26 2:58 p.m., Joe Slater wrote:
>> Pseudo tests for an item being renamed to itself only after
>> information about it has been deleted.  Move the test to before
>> we change the database.
>>
>> Note that pseudo does not support renameat2(), but neither does
>> glibc.
>>
>> Signed-off-by: Joe Slater <joe.slater@windriver.com>
>> ---
>>   .../pseudo/files/rename.patch                 | 73 +++++++++++++++++++
>>   meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
>>   2 files changed, 74 insertions(+)
>>   create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch
>>
>> diff --git a/meta/recipes-devtools/pseudo/files/rename.patch 
>> b/meta/recipes-devtools/pseudo/files/rename.patch
>> new file mode 100644
>> index 0000000000..bc344db3b5
>> --- /dev/null
>> +++ b/meta/recipes-devtools/pseudo/files/rename.patch
>> @@ -0,0 +1,73 @@
>> +pseudo: fix renaming to self
>> +
>> +The pseudo rename guts test for an item being renamed to
>> +itself, only after information about it has been deleted.
>> +We move the test to before we play with the database.
>> +
>> +Note that pseudo does not support renameat2().
>> +
>> +Upstream-Status: Pending
>> +
>> +Signed-off-by: Joe Slater <joe.slater@windriver.com>
>> +
>> +
>> +--- a/ports/unix/guts/rename.c
>> ++++ b/ports/unix/guts/rename.c
>> +@@ -29,6 +29,14 @@
>> +     newrc = base_lstat(newpath, &newbuf);
>> +     oldrc = base_lstat(oldpath, &oldbuf);
>> +
>> ++    /* nothing to do for a "rename" of a link to itself */
>> ++    if (newrc != -1 && oldrc != -1 &&
>> ++        newbuf.st_dev == oldbuf.st_dev &&
>> ++        newbuf.st_ino == oldbuf.st_ino) {
>> ++        pseudo_debug(PDBGF_OP, "rename: paths are the same\n");
>> ++        return real_rename(oldpath, newpath);
>> ++        }
>> ++
>> +     errno = save_errno;
>> +
>> +     /* newpath must be removed. */
>> +@@ -58,12 +66,6 @@
>> +         return rc;
>> +     }
>> +     save_errno = errno;
>> +-    /* nothing to do for a "rename" of a link to itself */
>> +-    if (newrc != -1 && oldrc != -1 &&
>> +-        newbuf.st_dev == oldbuf.st_dev &&
>> +-        newbuf.st_ino == oldbuf.st_ino) {
>> +-        return rc;
>> +-        }
>> +
>> +     /* rename(3) is not mv(1).  rename(file, dir) fails; you must 
>> provide
>> +      * the corrected path yourself.  You can rename over a 
>> directory only
>> +--- a/ports/unix/guts/renameat.c
>> ++++ b/ports/unix/guts/renameat.c
>> +@@ -41,6 +41,14 @@
>> +     newrc = base_fstatat(newdirfd, newpath, &newbuf, 
>> AT_SYMLINK_NOFOLLOW);
>> + #endif
>> +
>> ++    /* nothing to do for a "rename" of a link to itself */
>> ++    if (newrc != -1 && oldrc != -1 &&
>> ++        newbuf.st_dev == oldbuf.st_dev &&
>> ++        newbuf.st_ino == oldbuf.st_ino) {
>> ++        pseudo_debug(PDBGF_OP, "renameat: paths are the same\n");
>> ++        return real_renameat(olddirfd, oldpath, newdirfd, newpath);
>> ++        }
>> ++
>> +     errno = save_errno;
>> +
>> +     /* newpath must be removed. */
>> +@@ -71,12 +79,6 @@
>> +         return rc;
>> +     }
>> +     save_errno = errno;
>> +-    /* nothing to do for a "rename" of a link to itself */
>> +-    if (newrc != -1 && oldrc != -1 &&
>> +-        newbuf.st_dev == oldbuf.st_dev &&
>> +-        newbuf.st_ino == oldbuf.st_ino) {
>> +-        return rc;
>> +-        }
>> +
>> +     /* rename(3) is not mv(1).  rename(file, dir) fails; you must 
>> provide
>> +      * the corrected path yourself.  You can rename over a 
>> directory only
>> diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb 
>> b/meta/recipes-devtools/pseudo/pseudo_git.bb
>> index 9a22304bba..8d8cf8d523 100644
>> --- a/meta/recipes-devtools/pseudo/pseudo_git.bb
>> +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
>> @@ -4,6 +4,7 @@ SRC_URI = 
>> "git://git.yoctoproject.org/pseudo;branch=oe-core \
>>              file://0001-configure-Prune-PIE-flags.patch \
>>              file://fallback-passwd \
>>              file://fallback-group \
>> +           file://rename.patch \
>>              "
>>     SRCREV = "8efb082863ff0ceec7b7e46f9a44750e12f48039"
>
>

-- 
# Randy MacLeod
# Wind River Linux


[-- Attachment #2: Type: text/html, Size: 9107 bytes --]

Re: [oe-core][PATCH 1/1] pseudo: fix renaming to self

[Richard Purdie]

On Wed, 2020-08-26 at 11:58 -0700, Joe Slater wrote:
> Pseudo tests for an item being renamed to itself only after
> information about it has been deleted.  Move the test to before
> we change the database.
> 
> Note that pseudo does not support renameat2(), but neither does
> glibc.
> 
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>  .../pseudo/files/rename.patch                 | 73
> +++++++++++++++++++
>  meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
>  2 files changed, 74 insertions(+)
>  create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch

What I'll probably do is queue this to test, but then merge the patch
to the branch in the pseudo repo and we can just update the SRCREV
rather than more patches if things test ok.

Cheers,

Richard

Re: [oe-core][PATCH 1/1] pseudo: fix renaming to self

[Joe Slater]

Since I (accidently) enabled the patch for pseudo-native as well as target, the fact that builds still work and targets boot is a good sign.  Beyond that, I put pseudo on a target and verified the python3 rename.

For renameat(), I wrote a test I've attached to the Bugzilla case and compiled it in a devshell for pseudo-native.  Then, I used bin/pseudo in that devshell to verify pseudo does not forget about renames to self.

Assuming there is no gotcha, testing for rename identity before accessing the pseudo server saves many cycles.  Too bad renaming something to itself never happens.

-----Original Message-----
From: MacLeod, Randy <Randy.MacLeod@windriver.com> 
Sent: Wednesday, August 26, 2020 1:05 PM
To: Slater, Joseph <joe.slater@windriver.com>; openembedded-core@lists.openembedded.org; Seebs <seebs@seebs.net>
Subject: Re: [oe-core][PATCH 1/1] pseudo: fix renaming to self


Add Seebs.

Joe,
How did you test this change?

../Randy

On 2020-08-26 2:58 p.m., Joe Slater wrote:
> Pseudo tests for an item being renamed to itself only after 
> information about it has been deleted.  Move the test to before we 
> change the database.
>
> Note that pseudo does not support renameat2(), but neither does glibc.
>
> Signed-off-by: Joe Slater <joe.slater@windriver.com>
> ---
>   .../pseudo/files/rename.patch                 | 73 +++++++++++++++++++
>   meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
>   2 files changed, 74 insertions(+)
>   create mode 100644 meta/recipes-devtools/pseudo/files/rename.patch
>
> diff --git a/meta/recipes-devtools/pseudo/files/rename.patch 
> b/meta/recipes-devtools/pseudo/files/rename.patch
> new file mode 100644
> index 0000000000..bc344db3b5
> --- /dev/null
> +++ b/meta/recipes-devtools/pseudo/files/rename.patch
> @@ -0,0 +1,73 @@
> +pseudo: fix renaming to self
> +
> +The pseudo rename guts test for an item being renamed to itself, only 
> +after information about it has been deleted.
> +We move the test to before we play with the database.
> +
> +Note that pseudo does not support renameat2().
> +
> +Upstream-Status: Pending
> +
> +Signed-off-by: Joe Slater <joe.slater@windriver.com>
> +
> +
> +--- a/ports/unix/guts/rename.c
> ++++ b/ports/unix/guts/rename.c
> +@@ -29,6 +29,14 @@
> + 	newrc = base_lstat(newpath, &newbuf);
> + 	oldrc = base_lstat(oldpath, &oldbuf);
> +
> ++	/* nothing to do for a "rename" of a link to itself */
> ++	if (newrc != -1 && oldrc != -1 &&
> ++	    newbuf.st_dev == oldbuf.st_dev &&
> ++	    newbuf.st_ino == oldbuf.st_ino) {
> ++		pseudo_debug(PDBGF_OP, "rename: paths are the same\n");
> ++		return real_rename(oldpath, newpath);
> ++        }
> ++
> + 	errno = save_errno;
> +
> + 	/* newpath must be removed. */
> +@@ -58,12 +66,6 @@
> + 		return rc;
> + 	}
> + 	save_errno = errno;
> +-	/* nothing to do for a "rename" of a link to itself */
> +-	if (newrc != -1 && oldrc != -1 &&
> +-	    newbuf.st_dev == oldbuf.st_dev &&
> +-	    newbuf.st_ino == oldbuf.st_ino) {
> +-		return rc;
> +-        }
> +
> + 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
> + 	 * the corrected path yourself.  You can rename over a directory 
> +only
> +--- a/ports/unix/guts/renameat.c
> ++++ b/ports/unix/guts/renameat.c
> +@@ -41,6 +41,14 @@
> + 	newrc = base_fstatat(newdirfd, newpath, &newbuf, 
> +AT_SYMLINK_NOFOLLOW);  #endif
> +
> ++	/* nothing to do for a "rename" of a link to itself */
> ++	if (newrc != -1 && oldrc != -1 &&
> ++	    newbuf.st_dev == oldbuf.st_dev &&
> ++	    newbuf.st_ino == oldbuf.st_ino) {
> ++		pseudo_debug(PDBGF_OP, "renameat: paths are the same\n");
> ++		return real_renameat(olddirfd, oldpath, newdirfd, newpath);
> ++        }
> ++
> + 	errno = save_errno;
> +
> + 	/* newpath must be removed. */
> +@@ -71,12 +79,6 @@
> + 		return rc;
> + 	}
> + 	save_errno = errno;
> +-	/* nothing to do for a "rename" of a link to itself */
> +-	if (newrc != -1 && oldrc != -1 &&
> +-	    newbuf.st_dev == oldbuf.st_dev &&
> +-	    newbuf.st_ino == oldbuf.st_ino) {
> +-		return rc;
> +-        }
> +
> + 	/* rename(3) is not mv(1).  rename(file, dir) fails; you must provide
> + 	 * the corrected path yourself.  You can rename over a directory 
> + only
> diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb 
> b/meta/recipes-devtools/pseudo/pseudo_git.bb
> index 9a22304bba..8d8cf8d523 100644
> --- a/meta/recipes-devtools/pseudo/pseudo_git.bb
> +++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
> @@ -4,6 +4,7 @@ SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
>              file://0001-configure-Prune-PIE-flags.patch \
>              file://fallback-passwd \
>              file://fallback-group \
> +           file://rename.patch \
>              "
>   
>   SRCREV = "8efb082863ff0ceec7b7e46f9a44750e12f48039"


--
# Randy MacLeod
# Wind River Linux