[PATCH] state: check return value of chown
yeholmes at outlook.com
yeholmes at outlook.com
Sun Jan 3 01:19:19 EST 2021
From: Ye Holmes <yeholmes at outlook.com>
wrap around chown with `assert, this avoids unused result
warnings being treated as errors by prebuilt GCC (with
glibc-2.31, version 10.2):
procd-2020-12-12-7f12c89d/state.c: In function 'state_enter':
procd-2020-12-12-7f12c89d/state.c:147:4: error: ignoring
return value of 'chown' declared with attribute
'warn_unused_result' [-Werror=unused-result]
147 | chown(p->pw_dir, p->pw_uid, p->pw_gid);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Signed-off-by: Ye Holmes <yeholmes at outlook.com>
---
Thanks for your note, I will keep in mind that not to
increase code size by introducing unnecessary strings.
I've verified adding `assert resolves the error.
>On Sat, Jan 02, 2021 at 10:01:36PM +0800, yeholmes at outlook.com wrote:
>> From: Ye Holmes <yeholmes at outlook.com>
>>
>> Output warning in case changing the ownership of ubus' home
>> directory has failed. Besides, this avoids treating unused
>> result warning as error by GCC (with glibc-2.31, version 10.2):
>>
>> procd-2020-12-12-7f12c89d/state.c: In function 'state_enter':
>> procd-2020-12-12-7f12c89d/state.c:147:4: error: ignoring
>> return value of 'chown' declared with attribute
>> 'warn_unused_result' [-Werror=unused-result]
>> 147 | chown(p->pw_dir, p->pw_uid, p->pw_gid);
>> | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>
>> Signed-off-by: Ye Holmes <yeholmes at outlook.com>
>> ---
>> state.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/state.c b/state.c
>> index 44f56c6..d68776f 100644
>> --- a/state.c
>> +++ b/state.c
>> @@ -144,7 +144,8 @@ static void state_enter(void)
>> if (p) {
>> LOG("- ubus -\n");
>> mkdir(p->pw_dir, 0755);
>> - chown(p->pw_dir, p->pw_uid, p->pw_gid);
>> + if (chown(p->pw_dir, p->pw_uid, p->pw_gid))
>> + fprintf(stderr, "Failed to change ownership for %s\n", p->pw_dir);
>
>Please let's not have a custom error message for cases which
>practically never occur. If we would really cover all that, around
>80% of the size of executables like procd would be error messages.
>Imho an assertion is the right thing to do here.
>
>> } else {
>> LOG("- ubus (running as root!) -\n");
>> }
>> --
>> 2.25.1
---
state.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/state.c b/state.c
index 44f56c6..e9f101f 100644
--- a/state.c
+++ b/state.c
@@ -12,6 +12,7 @@
* GNU General Public License for more details.
*/
+#include <assert.h>
#include <fcntl.h>
#include <pwd.h>
#include <sys/reboot.h>
@@ -144,7 +145,7 @@ static void state_enter(void)
if (p) {
LOG("- ubus -\n");
mkdir(p->pw_dir, 0755);
- chown(p->pw_dir, p->pw_uid, p->pw_gid);
+ assert(chown(p->pw_dir, p->pw_uid, p->pw_gid) == 0);
} else {
LOG("- ubus (running as root!) -\n");
}
--
2.25.1
More information about the openwrt-devel
mailing list