Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{cut,diff,head,join,patch,xargs}: use getline() instead of fgetln() #893

Closed
wants to merge 6 commits into from

Conversation

arp242
Copy link
Contributor

@arp242 arp242 commented Nov 9, 2023

This replaces various uses of fgetln() with getline(). The main reason for this is portability, making things easier for people who want to compile these tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools, but fgetln() is impossible to port to most platforms, as concurrent access is essentially impossible to implement fully correct without the line buffer on the FILE struct. Other than this, many generic FreeBSD tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with 69099ba), and there's been some previous patches (ee3ca71 8c98e6b 1a2a4fc) for other tools.

Obtained from: https://github.com/dcantrell/bsdutils and
https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij martin@arp242.net

Copy link
Contributor

@igalic igalic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please split this into one commit for each sys system, one for cut, diff, head, join, patch, xargs

and add to their commit message a reference to this pull request:

Pull Request: https://github.com/freebsd/freebsd-src/pull/893

This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Pull Request:	freebsd#893
Signed-off-by: Martin Tournoij <martin@arp242.net>
@arp242
Copy link
Contributor Author

arp242 commented Nov 10, 2023

Sure – I changed it!

@bsdimp
Copy link
Member

bsdimp commented Dec 26, 2023

So these changes look good.

I'm none-the-less nervous about them. What is the useage of the altenrative bsd tools that you pointed me to? Have you run the kyua tests on these programs before/after to make sure that there's no subtle regressions?

@arp242
Copy link
Contributor Author

arp242 commented Dec 30, 2023

Have you run the kyua tests on these programs before/after to make sure that there's no subtle regressions?

I had assumed the CI would run this, but I gather it doesn't?

I've had great difficulty actually running the atf/kyua tests; it's been a while and I don't quite recall if I managed to get it working – I just remember a great deal of frustration.

@bsdimp bsdimp self-assigned this Apr 17, 2024
@bsdimp
Copy link
Member

bsdimp commented Apr 17, 2024

Have you run the kyua tests on these programs before/after to make sure that there's no subtle regressions?

I had assumed the CI would run this, but I gather it doesn't?

I've had great difficulty actually running the atf/kyua tests; it's been a while and I don't quite recall if I managed to get it working – I just remember a great deal of frustration.

(sorry missed this) I'll see if I can run kyua for these tests to see if there's no change.

@bsdimp
Copy link
Member

bsdimp commented Apr 19, 2024

ci run results: all specific tests in /usr/tests/usr.sbin/{head,cut,diff,patch,join,xargs} pass

@bsdimp
Copy link
Member

bsdimp commented Apr 19, 2024

5fbdcd6 pushed

@bsdimp bsdimp closed this Apr 19, 2024
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
freebsd-git pushed a commit that referenced this pull request Apr 19, 2024
This replaces fgetln() with getline(). The main reason for this is
portability, making things easier for people who want to compile these
tools on non-FreeBSD systems.

I appreciate that's probably not the top concern for FreeBSD base tools,
but fgetln() is impossible to port to most platforms, as concurrent
access is essentially impossible to implement fully correct without the
line buffer on the FILE struct. Other than this, many generic FreeBSD
tools compile fairly cleanly on Linux with a few small changes.

Most uses of fgetln() pre-date getline() support (added in 2009 with
69099ba), and there's been some previous patches (ee3ca71
8c98e6b 1a2a4fc) for other tools.

Obtained from:	https://github.com/dcantrell/bsdutils and
              	https://github.com/chimera-linux/chimerautils
Signed-off-by: Martin Tournoij <martin@arp242.net>
Reviewed by: imp
Pull Request: #893
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants