From nobody Wed Aug 3 20:59:15 2022 X-Original-To: dev-commits-src-main@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4LykkH5B5sz4XlCv for ; Wed, 3 Aug 2022 20:59:19 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: from mail-oi1-f173.google.com (mail-oi1-f173.google.com [209.85.167.173]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1D4" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4LykkG5Q44z3NR4 for ; Wed, 3 Aug 2022 20:59:18 +0000 (UTC) (envelope-from jrtc27@jrtc27.com) Received: by mail-oi1-f173.google.com with SMTP id v203so499525oie.10 for ; Wed, 03 Aug 2022 13:59:18 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=to:references:message-id:content-transfer-encoding:cc:date :in-reply-to:from:subject:mime-version:x-gm-message-state:from:to:cc; bh=IRMMIh6wzCTQYtC6KJ2ier5O+dO952Q2nJthfbgApLA=; b=t8InLTc4pNGiy2C/2akd3Pz7DP2EsvEeoLtW5PgkSf053qhDSuLQKc7kiwNgqGY26F /tFle72ly/15fa+utrgm2cs/G8KzTSipGk2+8hyPU+Qn0GqRtvoJyZQLmKgLulASY/xk qXZO4Uk+ARIi8kQxdaZ4wfZYwRqVZOYt2BEvypfmA68a40UZZYQwqPCzJ2H7epQ3xVAB 3V5cxPtoyb1N+PTQXRzLJHdrMmw3f0mXNrwk0H81UbWIQgSmJSD6BV6R6yriZ5y10ukv 5hIw/maegQInjI+8OkBfrLFh2wy9G5wZP0w8QsMl8NQUPN1vM4Gyxd9GkCHYdBG/LGm3 3Hmw== X-Gm-Message-State: ACgBeo0yiaDM/Br6gmldMQm/KbB/JtVAO9Ey54uD1rPq6UeIfujl5Cnd UETiGAiFcr3yQ0fGgdXGOkyrqA== X-Google-Smtp-Source: AA6agR6BWRtkvMdOMzXsPy6Si8KYbZm2am0OL79OZMiyj5vMVt2e3eNbYP95CtXnSgXPoYRPa9QirA== X-Received: by 2002:aca:1b06:0:b0:33a:6426:5cb8 with SMTP id b6-20020aca1b06000000b0033a64265cb8mr2441142oib.94.1659560357606; Wed, 03 Aug 2022 13:59:17 -0700 (PDT) Received: from smtpclient.apple (174-24-117-39.clsp.qwest.net. [174.24.117.39]) by smtp.gmail.com with ESMTPSA id m1-20020a9d6441000000b0061c7ec80898sm4279699otl.23.2022.08.03.13.59.16 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Wed, 03 Aug 2022 13:59:16 -0700 (PDT) Content-Type: text/plain; charset=utf-8 List-Id: Commit messages for the main branch of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-main List-Help: List-Post: List-Subscribe: List-Unsubscribe: Sender: owner-dev-commits-src-main@freebsd.org X-BeenThere: dev-commits-src-main@freebsd.org Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.80.82.1.1\)) Subject: Re: git: 36d67475f549 - main - xinstall: fix dounpriv logic, add tests From: Jessica Clarke In-Reply-To: <202208031904.273J4SuL021844@gitrepo.freebsd.org> Date: Wed, 3 Aug 2022 14:59:15 -0600 Cc: "src-committers@freebsd.org" , "dev-commits-src-all@freebsd.org" , "dev-commits-src-main@freebsd.org" Content-Transfer-Encoding: quoted-printable Message-Id: <87AA26D1-D894-4779-B965-ECD53C581516@freebsd.org> References: <202208031904.273J4SuL021844@gitrepo.freebsd.org> To: =?utf-8?Q?Dag-Erling_Sm=C3=B8rgrav?= X-Mailer: Apple Mail (2.3696.80.82.1.1) X-Rspamd-Queue-Id: 4LykkG5Q44z3NR4 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of jrtc27@jrtc27.com designates 209.85.167.173 as permitted sender) smtp.mailfrom=jrtc27@jrtc27.com X-Spamd-Result: default: False [-2.50 / 15.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; NEURAL_HAM_MEDIUM(-1.00)[-1.000]; NEURAL_HAM_SHORT(-1.00)[-1.000]; MV_CASE(0.50)[]; FORGED_SENDER(0.30)[jrtc27@freebsd.org,jrtc27@jrtc27.com]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17]; MIME_GOOD(-0.10)[text/plain]; RCPT_COUNT_THREE(0.00)[4]; PREVIOUSLY_DELIVERED(0.00)[dev-commits-src-main@freebsd.org]; TO_DN_EQ_ADDR_SOME(0.00)[]; DMARC_NA(0.00)[freebsd.org]; FROM_HAS_DN(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[209.85.167.173:from]; TO_MATCH_ENVRCPT_SOME(0.00)[]; FREEFALL_USER(0.00)[jrtc27]; MIME_TRACE(0.00)[0:+]; MLMMJ_DEST(0.00)[dev-commits-src-main@freebsd.org]; RCVD_COUNT_THREE(0.00)[3]; TO_DN_SOME(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; ARC_NA(0.00)[]; RCVD_TLS_LAST(0.00)[]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; R_DKIM_NA(0.00)[]; FROM_NEQ_ENVFROM(0.00)[jrtc27@freebsd.org,jrtc27@jrtc27.com]; MID_RHS_MATCH_FROM(0.00)[]; RWL_MAILSPIKE_POSSIBLE(0.00)[209.85.167.173:from] X-ThisMailContainsUnwantedMimeParts: N On 3 Aug 2022, at 13:04, Dag-Erling Sm=C3=B8rgrav = 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 > AuthorDate: 2022-08-03 18:59:28 +0000 > Commit: Dag-Erling Sm=C3=B8rgrav > 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) {