termux / termux-packages

A package build system for Termux.
https://termux.dev
Other
13.07k stars 3k forks source link

[Bug]: futimesat extra argument #19601

Closed algorythmic closed 6 months ago

algorythmic commented 6 months ago

Problem description

futimesat was changed in fbc59da0 (and https://github.com/termux/libandroid-utimes/commit/5316690c7452ff71f1bcea93cccf5be142818196) from the usual 3-argument form to one with an extra argument.

I'm not sure what the motivation was for adding the extra argument, but this (obviously) breaks any existing code that uses futimesat.

For example, building GNU coreutils fails with:

lib/utimens.c:420:44: error: too few arguments to function call, expected 4, have 3

What steps will reproduce the bug?

Try building code that uses futimesat as it is documented.

What is the expected behavior?

No response

System information

termux-info:

Termux Variables:
TERMUX_API_APP__VERSION_NAME=0.50.1+4c6a519
TERMUX_APP_PACKAGE_MANAGER=apt
TERMUX_APP__AM_SOCKET_SERVER_ENABLED=true
TERMUX_APP__APK_PATH=/data/app/~~c9neUff7nQJC82jmi0uE0A==/com.termux-CgY1EOjlG_80IDwyZO2xHA==/base.apk
TERMUX_APP__APK_RELEASE=GITHUB
TERMUX_APP__FILES_DIR=/data/user/0/com.termux/files
TERMUX_APP__IS_DEBUGGABLE_BUILD=true
TERMUX_APP__IS_INSTALLED_ON_EXTERNAL_STORAGE=false
TERMUX_APP__PACKAGE_MANAGER=apt
TERMUX_APP__PACKAGE_NAME=com.termux
TERMUX_APP__PACKAGE_VARIANT=apt-android-7
TERMUX_APP__PID=28010
TERMUX_APP__SE_FILE_CONTEXT=u:object_r:app_data_file:s0:c165,c258,c512,c768
TERMUX_APP__SE_INFO=default:targetSdkVersion=28:complete
TERMUX_APP__SE_PROCESS_CONTEXT=u:r:untrusted_app_27:s0:c165,c258,c512,c768
TERMUX_APP__TARGET_SDK=28
TERMUX_APP__UID=10677
TERMUX_APP__USER_ID=0
TERMUX_APP__VERSION_CODE=118
TERMUX_APP__VERSION_NAME=0.118.0+55cdef0
TERMUX_MAIN_PACKAGE_FORMAT=debian
TERMUX_VERSION=0.118.0+55cdef0
TERMUX__USER_ID=0
Packages CPU architecture:
aarch64
Subscribed repositories:
# sources.list
deb https://packages.termux.dev/apt/termux-main stable main
# root-repo (sources.list.d/root.list)
deb https://packages.termux.dev/apt/termux-root root stable
# x11-repo (sources.list.d/x11.list)
deb https://packages.termux.dev/apt/termux-x11 x11 main
Updatable packages:
All packages up to date
termux-tools version:
1.41.2
Android version:
14
Kernel build information:
Linux localhost 5.10.168-android12-9-... #1 SMP PREEMPT Wed Feb 28 07:18:02 UTC 2024 aarch64 GNU/Linux
Device manufacturer:
samsung
Device model:
SM-S906U
LD Variables:
LD_LIBRARY_PATH=
LD_PRELOAD=/data/data/com.termux/files/usr/lib/libtermux-exec.so
Installed termux plugins:
com.termux.x11 versionCode:14
com.termux.api versionCode:51
com.termux.styling versionCode:31
Grimler91 commented 6 months ago

Thanks for the report! The motivation is described in the commit you linked to, but we definetly need to add the standard version as well then!

algorythmic commented 6 months ago

The futimesat function takes an extra argument compared to bionic one: int flags, due to that unar expects that type of futimesat function

AFAICT the unar package just needs lutimes, and to implement this, the termux-added sys_time.c used a static 4-argument futimesat function:

https://github.com/termux/termux-packages/blob/f5fb85c7e3e4b2f2ab48ce788e985a376990c557/packages/unar/sys_time.c#L49-L60

i.e. no upstream code actually expects a 4-argument form.

Grimler91 commented 6 months ago

@algorythmic Yeah, you are right.

Should be fixed by https://github.com/termux/termux-packages/pull/19615.

algorythmic commented 6 months ago

Thanks! Though I think the change of argument type for lutimes in https://github.com/termux/libandroid-utimes/commit/155baa814a96c4888253c5b182144483a6f1a82e isn't quite right.

lutimes takes a timeval. If it is to be implemented via utimensat, it needs to convert to a timespec.

Grimler91 commented 6 months ago

Right, was misled by some compilation warnings there.

Fixed in https://github.com/termux/libandroid-utimes/commit/5e813795e9288cf09ef949355731bd89b7839fd8 (hopefully)