Date: Wed, 3 Aug 2022 14:59:15 -0600 From: Jessica Clarke <jrtc27@freebsd.org> To: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= <des@freebsd.org> Cc: "src-committers@freebsd.org" <src-committers@FreeBSD.org>, "dev-commits-src-all@freebsd.org" <dev-commits-src-all@FreeBSD.org>, "dev-commits-src-main@freebsd.org" <dev-commits-src-main@FreeBSD.org> Subject: Re: git: 36d67475f549 - main - xinstall: fix dounpriv logic, add tests Message-ID: <87AA26D1-D894-4779-B965-ECD53C581516@freebsd.org> In-Reply-To: <202208031904.273J4SuL021844@gitrepo.freebsd.org> References: <202208031904.273J4SuL021844@gitrepo.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On 3 Aug 2022, at 13:04, Dag-Erling Sm=C3=B8rgrav <des@freebsd.org> = wrote: >=20 > The branch main has been updated by des: >=20 > URL: = https://cgit.FreeBSD.org/src/commit/?id=3D36d67475f5497664d33c41c2f6745dcb= 30b0ec42 >=20 > commit 36d67475f5497664d33c41c2f6745dcb30b0ec42 > Author: Dag-Erling Sm=C3=B8rgrav <des@FreeBSD.org> > AuthorDate: 2022-08-03 18:59:28 +0000 > Commit: Dag-Erling Sm=C3=B8rgrav <des@FreeBSD.org> > CommitDate: 2022-08-03 19:03:49 +0000 >=20 > xinstall: fix dounpriv logic, add tests This is quite a poor commit message. What was wrong with it? Especially when the diff is cluttered with reformatting. I cannot obviously see any behavioural changes, just some changes from & to && that I don=E2=80=99= t believe technically matter, even if poor practice and not intended. Jess > Sponsored by: Klara, Inc. > MFC after: 1 week > --- > usr.bin/xinstall/tests/install_test.sh | 80 = ++++++++++++++++++++++++++++++++++ > usr.bin/xinstall/xinstall.c | 15 +++---- > 2 files changed, 87 insertions(+), 8 deletions(-) >=20 > diff --git a/usr.bin/xinstall/tests/install_test.sh = b/usr.bin/xinstall/tests/install_test.sh > index c723a6e0d280..e95f40fc19e7 100755 > --- a/usr.bin/xinstall/tests/install_test.sh > +++ b/usr.bin/xinstall/tests/install_test.sh > @@ -404,6 +404,84 @@ symbolic_link_relative_absolute_common_body() { > fi > } >=20 > +atf_test_case set_owner_group_mode > +set_owner_group_mode_head() { > + atf_set "require.user" "root" > +} > +set_owner_group_mode_body() { > + local fu=3D65531 fg=3D65531 > + local cu=3D65532 cg=3D65532 > + local u=3D"$(id -u)" > + local g=3D"$(id -g)" > + local m=3D0755 cm=3D4444 > + printf "test" >testf > + atf_check chown "$fu:$fg" testf > + atf_check chmod "$m" testf > + > + atf_check install testf testc > + atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -o "$cu" testf testc > + atf_check_equal "$cu:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -g "$cg" testf testc > + atf_check_equal "$u:$cg:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -o "$cu" -g "$cg" testf testc > + atf_check_equal "$cu:$cg:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -m "$cm" testf testc > + atf_check_equal "$u:$g:10$cm" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -o "$cu" -m "$cm" testf testc > + atf_check_equal "$cu:$g:10$cm" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -g "$cg" -m "$cm" testf testc > + atf_check_equal "$u:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -o "$cu" -g "$cg" -m "$cm" testf testc > + atf_check_equal "$cu:$cg:10$cm" "$(stat -f"%u:%g:%p" testc)" > +} > + > +atf_test_case set_owner_group_mode_unpriv > +set_owner_group_mode_unpriv_head() { > + atf_set "require.user" "root" > +} > +set_owner_group_mode_unpriv_body() { > + local fu=3D65531 fg=3D65531 > + local cu=3D65532 cg=3D65532 > + local u=3D"$(id -u)" > + local g=3D"$(id -g)" > + local m=3D0755 cm=3D4444 cM=3D0444 > + printf "test" >testf > + atf_check chown "$fu:$fg" testf > + atf_check chmod "$m" testf > + > + atf_check install -U testf testc > + atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -o "$cu" testf testc > + atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -g "$cg" testf testc > + atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -o "$cu" -g "$cg" testf testc > + atf_check_equal "$u:$g:10$m" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -m "$cm" testf testc > + atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -o "$cu" -m "$cm" testf testc > + atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -g "$cg" -m "$cm" testf testc > + atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)" > + > + atf_check install -U -o "$cu" -g "$cg" -m "$cm" testf testc > + atf_check_equal "$u:$g:10$cM" "$(stat -f"%u:%g:%p" testc)" > +} > + > atf_init_test_cases() { > atf_add_test_case copy_to_nonexistent > atf_add_test_case copy_to_nonexistent_safe > @@ -444,4 +522,6 @@ atf_init_test_cases() { > atf_add_test_case = symbolic_link_relative_absolute_source_and_dest2 > atf_add_test_case symbolic_link_relative_absolute_common > atf_add_test_case mkdir_simple > + atf_add_test_case set_owner_group_mode > + atf_add_test_case set_owner_group_mode_unpriv > } > diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c > index 05b1444506db..ddad7ba9115e 100644 > --- a/usr.bin/xinstall/xinstall.c > +++ b/usr.bin/xinstall/xinstall.c > @@ -1009,19 +1009,18 @@ install(const char *from_name, const char = *to_name, u_long fset, u_int flags) > #endif > } >=20 > - if (!dounpriv &=20 > - (gid !=3D (gid_t)-1 && gid !=3D to_sb.st_gid) || > - (uid !=3D (uid_t)-1 && uid !=3D to_sb.st_uid)) > + if (!dounpriv && ((gid !=3D (gid_t)-1 && gid !=3D to_sb.st_gid) = || > + (uid !=3D (uid_t)-1 && uid !=3D to_sb.st_uid))) { > if (fchown(to_fd, uid, gid) =3D=3D -1) { > serrno =3D errno; > (void)unlink(to_name); > errno =3D serrno; > err(EX_OSERR,"%s: chown/chgrp", to_name); > } > - > + } > if (mode !=3D (to_sb.st_mode & ALLPERMS)) { > if (fchmod(to_fd, > - dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : = mode)) { > + dounpriv ? mode & (S_IRWXU|S_IRWXG|S_IRWXO) : mode)) = { > serrno =3D errno; > (void)unlink(to_name); > errno =3D serrno; > @@ -1036,7 +1035,7 @@ install(const char *from_name, const char = *to_name, u_long fset, u_int flags) > * trying to turn off UF_NODUMP. If we're trying to set real = flags, > * then warn if the fs doesn't support it, otherwise fail. > */ > - if (!dounpriv & !devnull && (flags & SETFLAGS || > + if (!dounpriv && !devnull && (flags & SETFLAGS || > (from_sb.st_flags & ~UF_NODUMP) !=3D to_sb.st_flags) && > fchflags(to_fd, > flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) { > @@ -1082,7 +1081,7 @@ compare(int from_fd, const char *from_name = __unused, size_t from_len, > return 1; >=20 > do_digest =3D (digesttype !=3D DIGEST_NONE && dresp !=3D NULL && > - *dresp =3D=3D NULL); > + *dresp =3D=3D NULL); > if (from_len <=3D MAX_CMP_SIZE) { > if (do_digest) > digest_init(&ctx); > @@ -1390,7 +1389,7 @@ install_dir(char *path) > ch =3D *p; > *p =3D '\0'; > again: > - if (stat(path, &sb) < 0) { > + if (stat(path, &sb) !=3D 0) { > if (errno !=3D ENOENT || tried_mkdir) > err(EX_OSERR, "stat %s", path); > if (mkdir(path, 0755) < 0) {
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?87AA26D1-D894-4779-B965-ECD53C581516>