[2/2] pseudo: Fix statx function usage

Submitted by Richard Purdie on Oct. 7, 2020, 4:20 p.m. | Patch ID: 177038

Details

Message ID 20201007162018.3187947-2-richard.purdie@linuxfoundation.org
State Master Next
Commit 6ccfe307a21e9ae591aa8a71af8672017c6ecd47
Headers show

Commit Message

Richard Purdie Oct. 7, 2020, 4:20 p.m.
There is magic in the posts where specific variable names have specific
magic. For that magic to work, "path" needs to be used not "pathname" as
is currently there. Fix this, which fixes path issues on systems using
statx (Ubuntu 20.04 in particular).

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
---
 .../pseudo/files/statxfix.patch               | 56 +++++++++++++++++++
 meta/recipes-devtools/pseudo/pseudo_git.bb    |  1 +
 2 files changed, 57 insertions(+)
 create mode 100644 meta/recipes-devtools/pseudo/files/statxfix.patch

Patch hide | download patch | download mbox

diff --git a/meta/recipes-devtools/pseudo/files/statxfix.patch b/meta/recipes-devtools/pseudo/files/statxfix.patch
new file mode 100644
index 00000000000..c47ff27f9f4
--- /dev/null
+++ b/meta/recipes-devtools/pseudo/files/statxfix.patch
@@ -0,0 +1,56 @@ 
+There is magic in the posts where specific variable names have specific
+magic. For that magic to work, "path" needs to be used not "pathname" as
+is currently there. Fix this, which fixes path issues on systems using 
+statx (Ubuntu 20.04 in particular).
+
+Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
+Upstream-Status: Pending
+
+Index: git/ports/linux/statx/guts/statx.c
+===================================================================
+--- git.orig/ports/linux/statx/guts/statx.c
++++ git/ports/linux/statx/guts/statx.c
+@@ -5,14 +5,14 @@
+  * SPDX-License-Identifier: LGPL-2.1-only
+  *
+  * int
+- * statx(int dirfd, const char *pathname, int flags, unsigned int mask, struct statx *statxbuf) {
++ * statx(int dirfd, const char *path, int flags, unsigned int mask, struct statx *statxbuf) {
+  *	int rc = -1;
+  */
+ 	pseudo_msg_t *msg;
+ 	PSEUDO_STATBUF buf;
+ 	int save_errno;
+ 
+-	rc = real_statx(dirfd, pathname, flags, mask, statxbuf);
++	rc = real_statx(dirfd, path, flags, mask, statxbuf);
+ 	save_errno = errno;
+ 	if (rc == -1) {
+ 		return rc;
+@@ -25,16 +25,16 @@
+ 	buf.st_mode = statxbuf->stx_mode;
+ 	buf.st_rdev = makedev(statxbuf->stx_rdev_major, statxbuf->stx_rdev_minor);
+ 	buf.st_nlink = statxbuf->stx_nlink;
+-	msg = pseudo_client_op(OP_STAT, 0, -1, dirfd, pathname, &buf);
++	msg = pseudo_client_op(OP_STAT, 0, -1, dirfd, path, &buf);
+ 	if (msg && msg->result == RESULT_SUCCEED) {
+-		pseudo_debug(PDBGF_FILE, "statx(path %s), flags %o, stat rc %d, stat uid %o\n", pathname, flags, rc, statxbuf->stx_uid);
++		pseudo_debug(PDBGF_FILE, "statx(path %s), flags %o, stat rc %d, stat uid %o\n", path, flags, rc, statxbuf->stx_uid);
+ 		statxbuf->stx_uid = msg->uid;
+ 		statxbuf->stx_gid = msg->gid;
+ 		statxbuf->stx_mode = msg->mode;
+ 		statxbuf->stx_rdev_major = major(msg->rdev);
+ 		statxbuf->stx_rdev_minor = minor(msg->rdev);
+ 	} else {
+-		pseudo_debug(PDBGF_FILE, "statx(path %s) failed, flags %o, stat rc %d, stat uid %o\n", pathname, flags, rc, statxbuf->stx_uid);
++		pseudo_debug(PDBGF_FILE, "statx(path %s) failed, flags %o, stat rc %d, stat uid %o\n", path, flags, rc, statxbuf->stx_uid);
+ 	}
+ 	errno = save_errno;
+ /*	return rc;
+Index: git/ports/linux/statx/wrapfuncs.in
+===================================================================
+--- git.orig/ports/linux/statx/wrapfuncs.in
++++ git/ports/linux/statx/wrapfuncs.in
+@@ -1 +1 @@
+-int statx(int dirfd, const char *pathname, int flags, unsigned int mask, struct statx *statxbuf);
++int statx(int dirfd, const char *path, int flags, unsigned int mask, struct statx *statxbuf);
diff --git a/meta/recipes-devtools/pseudo/pseudo_git.bb b/meta/recipes-devtools/pseudo/pseudo_git.bb
index 90f5cb0bcf2..626e658f65d 100644
--- a/meta/recipes-devtools/pseudo/pseudo_git.bb
+++ b/meta/recipes-devtools/pseudo/pseudo_git.bb
@@ -10,6 +10,7 @@  SRC_URI = "git://git.yoctoproject.org/pseudo;branch=oe-core \
            file://mayunlink.patch \
            file://pathfix.patch \
            file://proc.patch \
+           file://statxfix.patch \
            file://fallback-passwd \
            file://fallback-group \
            "

Comments

Seebs Oct. 12, 2020, 7:08 p.m.
On Wed,  7 Oct 2020 17:20:18 +0100
"Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:

> +There is magic in the posts where specific variable names have
> specific +magic. For that magic to work, "path" needs to be used not
> "pathname" as +is currently there. Fix this, which fixes path issues
> on systems using +statx (Ubuntu 20.04 in particular).

So, usually if "pathname" is used, it's intentional to suppress the
special magic behavior. Usually, but not always. In this case, the man
page does use "pathname" and sometimes that means it's just a cut and
paste error.

My man page also says there's no glibc wrapper for statx, making me
slightly surprised that a wrapper for it is needed/relevant.

-s
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#143246): https://lists.openembedded.org/g/openembedded-core/message/143246
Mute This Topic: https://lists.openembedded.org/mt/77365704/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Mark Hatle Oct. 12, 2020, 7:14 p.m.
On 10/12/20 2:08 PM, Seebs wrote:
> On Wed,  7 Oct 2020 17:20:18 +0100
> "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:
> 
>> +There is magic in the posts where specific variable names have
>> specific +magic. For that magic to work, "path" needs to be used not
>> "pathname" as +is currently there. Fix this, which fixes path issues
>> on systems using +statx (Ubuntu 20.04 in particular).
> 
> So, usually if "pathname" is used, it's intentional to suppress the
> special magic behavior. Usually, but not always. In this case, the man
> page does use "pathname" and sometimes that means it's just a cut and
> paste error.
> 
> My man page also says there's no glibc wrapper for statx, making me
> slightly surprised that a wrapper for it is needed/relevant.

I looked into this.  It appears (after pseudo was written, but) about 7 or so
years ago, all of the man pages were re-written and the language was changed.

So 'pathname' no longer has the same syntactic meaning that it did when pseudo
was first written.  (It's not just statx, I looked at things like fopen, open, etc.)

--Mark

> -s
> 
> 
> 
> 
>
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#143247): https://lists.openembedded.org/g/openembedded-core/message/143247
Mute This Topic: https://lists.openembedded.org/mt/77365704/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Seebs Oct. 12, 2020, 7:21 p.m.
On Mon, 12 Oct 2020 14:14:36 -0500
"Mark Hatle" <mark.hatle@kernel.crashing.org> wrote:

> I looked into this.  It appears (after pseudo was written, but) about
> 7 or so years ago, all of the man pages were re-written and the
> language was changed.

I don't think I originally intentionally synchronized it with man
pages, I just had the convention of special-casing "path", and then
started using "pathname" as a convention for "a name that holds a
path but we don't want to do the magic on it".

-s
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#143249): https://lists.openembedded.org/g/openembedded-core/message/143249
Mute This Topic: https://lists.openembedded.org/mt/77365704/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-
Richard Purdie Oct. 12, 2020, 8:25 p.m.
On Mon, 2020-10-12 at 14:08 -0500, Seebs wrote:
> On Wed,  7 Oct 2020 17:20:18 +0100
> "Richard Purdie" <richard.purdie@linuxfoundation.org> wrote:
> 
> > +There is magic in the posts where specific variable names have
> > specific +magic. For that magic to work, "path" needs to be used
> > not
> > "pathname" as +is currently there. Fix this, which fixes path
> > issues
> > on systems using +statx (Ubuntu 20.04 in particular).
> 
> So, usually if "pathname" is used, it's intentional to suppress the
> special magic behavior.

In this case I added the statx wrapper and didn't realise there was
magic related to the name "path", so its a bug I introduced. This patch
fixes my mistake. I think these are only on the oe-core branch, not
master.

>  Usually, but not always. In this case, the man
> page does use "pathname" and sometimes that means it's just a cut and
> paste error.

Its my own copy and paste bug :)

> My man page also says there's no glibc wrapper for statx, making me
> slightly surprised that a wrapper for it is needed/relevant.

Its definitely being used by Ubuntu 20.04 systems as we're findining
bugs related to it!

Thanks for the review!

Cheers,

Richard
-=-=-=-=-=-=-=-=-=-=-=-
Links: You receive all messages sent to this group.
View/Reply Online (#143255): https://lists.openembedded.org/g/openembedded-core/message/143255
Mute This Topic: https://lists.openembedded.org/mt/77365704/3617530
Group Owner: openembedded-core+owner@lists.openembedded.org
Unsubscribe: https://lists.openembedded.org/g/openembedded-core/unsub [oe-patchwork@oe-patch.openembedded.org]
-=-=-=-=-=-=-=-=-=-=-=-