Details

Reviewers

Group Reviewers

Commits

rG88db004d3369: whereis(1): Fix style(9)
rGfc002631af3e: whereis(1): Respect PORTSDIR variable

Summary

Respect PORTSDIR variable for those who have the ports collection in a different
place than /usr/ports. PORTSDIR is a very common variable used in the ports
framework and in /etc/make.conf among other places.

Add note to manual page.
.Dd to be bumped at commit time.

Note that this same behavior can be achieved by doing something like:

export PORTSDIR=~/FreeBSD-repos/ports
whereis -S $PORTSDIR/** -f converseen

Assuming the shell is able to expand the **, but it is much less convenient.

Test Plan

Assuming the ports tree is in $HOME/FreeBSD-repos/ports and that there is no
/usr/ports, if we execute the following:

$ whereis converseen
converseen:

Apply the patch and run the following:

$ PORTSDIR=$HOME/FreeBSD-repos/ports ./whereis converseen
converseen: /home/fernape/FreeBSD-repos/ports/graphics/converseen

If we create the /usr/ports directory and populate some directories:

$ ls -R /usr/ports/
foo/      graphics/

/usr/ports/foo:
bar/

/usr/ports/foo/bar:

/usr/ports/graphics:
converseen/

/usr/ports/graphics/converseen:
Makefile   distinfo   pkg-descr  pkg-plist

and execute the same command we get the /usr/ports result as expected:

$ PORTSDIR=$HOME/FreeBSD-repos/ports ./whereis converseen
converseen: /usr/ports/graphics/converseen

Asking for all results gives both entries:

$ PORTSDIR=$HOME/FreeBSD-repos/ports ./whereis -a converseen
converseen: /usr/ports/graphics/converseen
/home/fernape/FreeBSD-repos/ports/graphics/converseen

Unsetting the variable works as expected:

$ ./whereis -a converseen
converseen: /usr/ports/graphics/converseen

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 76582
Build 73465: arc lint + arc unit

Event Timeline

fernape created this revision.Oct 11 2023, 11:12 AM

Herald added a subscriber: imp. · View Herald TranscriptOct 11 2023, 11:12 AM

fernape requested review of this revision.Oct 11 2023, 11:12 AM

Harbormaster completed remote builds in B53923: Diff 128545.Oct 11 2023, 11:12 AM

fernape edited the summary of this revision. (Show Details)Oct 11 2023, 11:13 AM

fernape added a reviewer: Src Committers.

patmaddox added a subscriber: patmaddox.Oct 19 2023, 9:44 PM

Ping!

Friendly ping

Another ping!

Herald added a subscriber: ziaee. · View Herald TranscriptAug 17 2025, 3:37 PM

Ping! 🔔

delphij added a subscriber: delphij.Sat, Aug 22, 4:17 PM

delphij added inline comments.

usr.bin/whereis/whereis.c
373–374	Why not simply using `b` here for the assignment? (For readability you might want to set `b` as `NULL` afterward to make it explicit that the ownership is changed, but doing `strdup()` here seems to be redundant, and OOM situation is not handled).

Remove spurious string copy.

While here fix indentation.

Harbormaster completed remote builds in B75986: Diff 184795.Sun, Aug 23, 3:48 PM

fernape marked an inline comment as done.Sun, Aug 23, 3:49 PM

fernape added inline comments.

usr.bin/whereis/whereis.c
373–374	Thanks for having a look at this. I don't see a reason to duplicate the string, and if at some point I did, I honestly do not remember. While here, let me fix the indentation in the `for` loop.

ngie added a subscriber: ngie.Sat, Aug 29, 3:06 PM

ngie added inline comments.

usr.bin/whereis/whereis.c
357
367–369
373
375–376	This feels unnecessary given the prior stat call.
397–399	Please use asprintf, etc, to build paths instead of 3 separate calls.

Address feedback by ngie@

Harbormaster completed remote builds in B76351: Diff 185486.Mon, Aug 31, 5:05 PM

fernape marked 5 inline comments as done.Mon, Aug 31, 5:07 PM

fernape added inline comments.

usr.bin/whereis/whereis.c
397–399	There are other (old) instances of this sequential triplet of `strcat` calls around. Let me know if you think we should change them as well.

ngie added inline comments.Mon, Aug 31, 5:21 PM

usr.bin/whereis/whereis.c
328	Where is `b` freed after this change?
378–382	Let's get rid of an antique CVS reference while here :).
387–389	This seems a bit suspect: you could technically set the locale to "C" to get this behavior and use `isupper` here. Not sure why the original author thought this was the best approach to take...
397–399	There are other (old) instances of this sequential triplet of `strcat` calls around. Let me know if you think we should change them as well. Sounds like a good junior cleanup task / opportunity for adding new tests (as needed!) :).

Try and fix leak issue.

Harbormaster completed remote builds in B76501: Diff 185828.Fri, Sep 4, 4:32 PM

fernape marked 2 inline comments as done.Fri, Sep 4, 4:35 PM

fernape added inline comments.

usr.bin/whereis/whereis.c
328	Let's do something different here. Do not touch `b` since unfortunately, `decolonify` keeps pointers to the original string and hence, we need to keep `b` around. Instead, use `pp` to build the port subdir path and copy it into `srcpaths`, then `free(pp)`.
387–389	No idea :-)

How about approaching this slightly differently? If ${PORTSDIR} is not defined in the environment, use /usr/ports. e.g., PORTSDIR?= /usr/ports (using make syntax). That drops the need for adding a for-loop to handle both cases and touching/reformatting a ton of code.

If the user passes a non-existent ${PORTSDIR} -- that's their mistake; don't contort too much trying to check for both scenarios (I assume that the number of folks that specify a ${PORTSDIR} != /usr/ports is likely small and they should know what they're doing if they specify the override).

In D42156#1362778, @ngie wrote:

How about approaching this slightly differently? If ${PORTSDIR} is not defined in the environment, use /usr/ports. e.g., PORTSDIR?= /usr/ports (using make syntax). That drops the need for adding a for-loop to handle both cases and touching/reformatting a ton of code.

If the user passes a non-existent ${PORTSDIR} -- that's their mistake; don't contort too much trying to check for both scenarios (I assume that the number of folks that specify a ${PORTSDIR} != /usr/ports is likely small and they should know what they're doing if they specify the override).

Doing what I suggest above also avoids the scenario where someone specifies PORTSDIR=/usr/ports in their environment (which would build a whereas database for both /usr/ports and ${PORTSDIR} with the proposed change).

It's better to handle the fork in the road logic as "either PORTSDIR or /usr/ports", not "both PORTSDIR and /usr/ports".

Use PORTSDIR and default to /usr/ports if empty.

Harbormaster completed remote builds in B76582: Diff 185949.Sat, Sep 5, 6:07 PM

ngie accepted this revision.Sun, Sep 6, 5:35 AM

ngie added inline comments.

usr.bin/whereis/whereis.c
319
326

This revision is now accepted and ready to land.Sun, Sep 6, 5:35 AM

Closed by commit rGfc002631af3e: whereis(1): Respect PORTSDIR variable (authored by fernape). · Explain WhySun, Sep 6, 3:28 PM

This revision was automatically updated to reflect the committed changes.

fernape added a commit: rGfc002631af3e: whereis(1): Respect PORTSDIR variable.

Thanks for the review!

fernape added a commit: rG88db004d3369: whereis(1): Fix style(9).Sun, Sep 6, 4:57 PM

whereis(1): Respect PORTSDIR variable
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 185949

usr.bin/whereis/whereis.1

usr.bin/whereis/whereis.c

whereis(1): Respect PORTSDIR variableClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 185949

usr.bin/whereis/whereis.1

usr.bin/whereis/whereis.c

whereis(1): Respect PORTSDIR variable
ClosedPublic
Actions

Revision Contents
Changeset List