From owner-cvs-all Fri Jul 27 22:35: 2 2001 Delivered-To: cvs-all@freebsd.org Received: from obsecurity.dyndns.org (adsl-64-169-104-149.dsl.lsan03.pacbell.net [64.169.104.149]) by hub.freebsd.org (Postfix) with ESMTP id 0FCB637B406; Fri, 27 Jul 2001 22:34:51 -0700 (PDT) (envelope-from kris@obsecurity.org) Received: by obsecurity.dyndns.org (Postfix, from userid 1000) id 35FFB66E8D; Fri, 27 Jul 2001 22:34:50 -0700 (PDT) Date: Fri, 27 Jul 2001 22:34:49 -0700 From: Kris Kennaway To: David O'Brien Cc: Kris Kennaway , cvs-committers@FreeBSD.org, cvs-all@FreeBSD.org Subject: Re: cvs commit: src/usr.sbin/pkg_install/add main.c src/usr.sbin/pkg_install/lib str.c Message-ID: <20010727223449.B60313@xor.obsecurity.org> References: <200107280159.f6S1xw810069@freefall.freebsd.org> <20010727204037.A58795@xor.obsecurity.org> <20010727214454.A96927@dragon.nuxi.com> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-md5; protocol="application/pgp-signature"; boundary="rJwd6BRFiFCcLxzm" Content-Disposition: inline User-Agent: Mutt/1.2.5i In-Reply-To: <20010727214454.A96927@dragon.nuxi.com>; from obrien@FreeBSD.org on Fri, Jul 27, 2001 at 09:44:54PM -0700 Sender: owner-cvs-all@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG --rJwd6BRFiFCcLxzm Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Fri, Jul 27, 2001 at 09:44:54PM -0700, David O'Brien wrote: > On Fri, Jul 27, 2001 at 08:40:37PM -0700, Kris Kennaway wrote: > > > Modified files: > > > usr.sbin/pkg_install/add main.c=20 > > > usr.sbin/pkg_install/lib str.c=20 > > > Log: > > > Remove s_strl*(). I am not sure what was thought they accomplished. > >=20 > > Grr. Thanks for discussing this with me (the person who added > > s_strl*). =20 > > > > I'm not happy with this commit: I don't like endless > > amounts of inline code duplication when a simple function can do the > > job. >=20 > You have a weird definition of "endless amounts of code duplication". > With your position, what do you call the errx() after every s_strl* call > then? Why didn't you fold that into s_strl*? If s_strl* contained that > then maybe you'd have an argument about code duplication. That would have been a possible improvement, yes, although my intention was eventually to fix other instances of string handling in pkg_* which weren't dealing with package name manipulation. You should have suggested the change to me by prior email. Perhaps the s_strl* code should also have been a macro. > Do you understand how compilers generate code? Have you ever written a > compiler? Do you know how optimizers work? Have you ever taken a > graduate code optimization course? Do you have any code in a widely used > compiler? Done any graduate work on processor architecture? Straw man. This code is not critical-path, and so a pessimization of a few processor cycles won't affect anyone. > > Please back this out until we can come to an agreement. >=20 > What part of my explanation for the commit do you disagree with? > I also showed the code before my commit and after my commit to some > coworkers. They agreed my version improved the clarity. You should have discussed it with me. I made that change for good reason; perhaps I was misguided, but the polite thing to do would have been to talk about it with me privately and make your suggestions instead of unilaterally making the commit and stomping on my change. I'm still willing to discuss it, although now we're in an emotionally charged, confrontational position and it's going to be harder to avoid shouting at each other. Kris --rJwd6BRFiFCcLxzm Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.0.6 (FreeBSD) Comment: For info see http://www.gnupg.org iD8DBQE7Yk75Wry0BWjoQKURAmPKAJ4oIL97g5X6G70vTZJQsU5KzzfcGQCg/iQA tjm90COkKAGjIM+HCVl6WvU= =yNqn -----END PGP SIGNATURE----- --rJwd6BRFiFCcLxzm-- To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe cvs-all" in the body of the message