Revert "patch: fix pch_context() for unified diffs with no leading context"

This reverts commit f97b6a8f84b3ed209c2aea0958a7b889d0bf27ed, as it
turns out our fuzz implementation is just too naive.

Revert "patch: fix pch_context() for unified diffs with no leading context"

This reverts commit f97b6a8f84b3ed209c2aea0958a7b889d0bf27ed, as it
turns out our fuzz implementation is just too naive. We can have more
leading context than trailing context and vice-versa, so we can't really
assume they're the same.

Restore the previous bug and we can work on it post-branch.

Reported by: cy

show more ...

patch: fix pch_context() for unified diffs with no leading context

When the first line of a file is a removal, we may not have any leading
context. Only adjusting p_context if context > 0 means tha

patch: fix pch_context() for unified diffs with no leading context

When the first line of a file is a removal, we may not have any leading
context. Only adjusting p_context if context > 0 means that we
incorrectly believe that we have 100 lines of context when the reality
is that we have none.

This fixes a bug with fuzz-checking, which ends up fuzzing away the line
we're trying to replace if it's the first line in the file. We use
pch_context() to determine a reasonable max-fuzz.

PR: 250511
Reviewed by: pfg
Differential Revision: https://reviews.freebsd.org/D51837

show more ...

patch: use getline() instead of fgetln()

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-FreeBS

patch: use getline() instead of fgetln()

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
69099ba2ec8b), and there's been some previous patches (ee3ca711a898
8c98e6b1a7f3 1a2a4fc8ce1b) 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: https://github.com/freebsd/freebsd-src/pull/893

show more ...

patch: Support long context lines.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: allanjude
Differential Revision: https://reviews.freebsd.org/D43850

Remove $FreeBSD$: one-line .h pattern

Remove /^\s*\*+\s*\$FreeBSD\$.*$\n/

patch: don't run off the end of path if it ends in '/'.

Found by fuzzing (afl) in OpenBSD.

Obtained from: OpenBSD (CVS 1.65)

patch: cleanup variable initialization a bit.

musl libc fgetln is a bit more pickier.

Hinted by: chimera-linux (git 31491e1de2e1241885984cd9e4b978965f14eda4)

MFH

Sponsored by: Rubicon Communications, LLC (netgate.com)

Remove heuristic for dealing with trailing newlines being truncated by mailers.

Every version of patch since the first one posted to mod.sources in 1985 have
included a heuristic for coping with the

Remove heuristic for dealing with trailing newlines being truncated by mailers.

Every version of patch since the first one posted to mod.sources in 1985 have
included a heuristic for coping with the state of email messaging at the
time. This heuristic would add up to 4 blank lines to a patch if it thought it
needed it. The trouble is, though this causes at least one bug.

The bug in my case is that if you have a context diff whose last hunk only
deletes 3 or fewer lines, then if you try to reverse apply it with -R, it will
fail. The reason for this is the heuristic builds an internal representation
that includes those blank lines. However, it should really replicate the lines
from the pattern lines line it would any other time, not assume they are blank
lines. Removing this heuristic will prevent patch from misapplying the lines
removed after applying a 'fuzz' factor to the previous blank line in the file. I
believe this will only affect 'new-style' 4.3BSD context diffs and not the
older-style 4.2BSD diffs and plain, non-context diffs. It won't affect any of
the newer formats, since they don't use the 'omitted' construct in the same way.

Since this heuristic was put into patch at a time when email / etc ate trailing
white space on a regular basis, and since it's clear that this heuristic is the
wrong thing to do at least some of the time, it's better to remove it
entirely. It's not been needed for maybe 20 years since patch files are not
usually corrupted. If there are a small number of patch files that would benefit
from this corruption fixing, those already-currupt patches can be fixed by the
addition of blank lines. I'd wager that no one will ever come to me with an
example of a once-working patch file that breaks with this change. However, I
have 2 patches from the first 195 patches to 2.11BSD that are affected by this
bug, suggesting that the relative frequency of the issue has changed
signficantly since the original heuristic was put into place.

Reviewed by: phk@
Differential Revision: https://reviews.freebsd.org/D26081

show more ...

patch(1): give /dev/null patches special treatment

We have a bad habit of duplicating contents of files that are sourced from
/dev/null and applied more than once... take the more sane (in most ways

patch(1): give /dev/null patches special treatment

We have a bad habit of duplicating contents of files that are sourced from
/dev/null and applied more than once... take the more sane (in most ways)
GNU route and complain if the file exists and offer reversal options.

This still falls short a little bit as selecting "don't reverse, apply
anyway" will still give you duplicated file contents. There's probably other
issues as well, but awareness is the first step to happiness.

MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D21535

show more ...

MFHead@r345275

MFhead@r344786

Merge ^/head r344549 through r344775.

patch(1): Exit successfully if we're fed a 0-length patch

This change is made in the name of GNU patch compatibility. If GNU patch is
fed a zero-length patch, it will exit successfully with no outpu

patch(1): Exit successfully if we're fed a 0-length patch

This change is made in the name of GNU patch compatibility. If GNU patch is
fed a zero-length patch, it will exit successfully with no output. This is
used in at least one port to date (comms/wsjtx), and we break on this usage.

It seems unlikely that anyone relies on patch(1) calling their completely
empty patch garbage and failing, and GNU compatibility is a plus if it helps
with porting, so make the switch.

Reported by: db
MFC after: 2 weeks

show more ...

Revert "patch: fix pch_context() for unified diffs with no leading context"

This reverts commit f97b6a8f84b3ed209c2aea0958a7b889d0bf27ed, as it
turns out our fuzz implementation is just too naive.

Revert "patch: fix pch_context() for unified diffs with no leading context"

This reverts commit f97b6a8f84b3ed209c2aea0958a7b889d0bf27ed, as it
turns out our fuzz implementation is just too naive. We can have more
leading context than trailing context and vice-versa, so we can't really
assume they're the same.

Restore the previous bug and we can work on it post-branch.

Reported by: cy

show more ...

patch: fix pch_context() for unified diffs with no leading context

When the first line of a file is a removal, we may not have any leading
context. Only adjusting p_context if context > 0 means tha

patch: fix pch_context() for unified diffs with no leading context

When the first line of a file is a removal, we may not have any leading
context. Only adjusting p_context if context > 0 means that we
incorrectly believe that we have 100 lines of context when the reality
is that we have none.

This fixes a bug with fuzz-checking, which ends up fuzzing away the line
we're trying to replace if it's the first line in the file. We use
pch_context() to determine a reasonable max-fuzz.

PR: 250511
Reviewed by: pfg
Differential Revision: https://reviews.freebsd.org/D51837

show more ...

patch: use getline() instead of fgetln()

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-FreeBS

patch: use getline() instead of fgetln()

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
69099ba2ec8b), and there's been some previous patches (ee3ca711a898
8c98e6b1a7f3 1a2a4fc8ce1b) 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: https://github.com/freebsd/freebsd-src/pull/893

show more ...

patch: Support long context lines.

MFC after: 1 week
Sponsored by: Klara, Inc.
Reviewed by: allanjude
Differential Revision: https://reviews.freebsd.org/D43850

Remove $FreeBSD$: one-line .h pattern

Remove /^\s*\*+\s*\$FreeBSD\$.*$\n/

patch: don't run off the end of path if it ends in '/'.

Found by fuzzing (afl) in OpenBSD.

Obtained from: OpenBSD (CVS 1.65)

patch: cleanup variable initialization a bit.

musl libc fgetln is a bit more pickier.

Hinted by: chimera-linux (git 31491e1de2e1241885984cd9e4b978965f14eda4)

MFH

Sponsored by: Rubicon Communications, LLC (netgate.com)

Remove heuristic for dealing with trailing newlines being truncated by mailers.

Every version of patch since the first one posted to mod.sources in 1985 have
included a heuristic for coping with the

Remove heuristic for dealing with trailing newlines being truncated by mailers.

Every version of patch since the first one posted to mod.sources in 1985 have
included a heuristic for coping with the state of email messaging at the
time. This heuristic would add up to 4 blank lines to a patch if it thought it
needed it. The trouble is, though this causes at least one bug.

The bug in my case is that if you have a context diff whose last hunk only
deletes 3 or fewer lines, then if you try to reverse apply it with -R, it will
fail. The reason for this is the heuristic builds an internal representation
that includes those blank lines. However, it should really replicate the lines
from the pattern lines line it would any other time, not assume they are blank
lines. Removing this heuristic will prevent patch from misapplying the lines
removed after applying a 'fuzz' factor to the previous blank line in the file. I
believe this will only affect 'new-style' 4.3BSD context diffs and not the
older-style 4.2BSD diffs and plain, non-context diffs. It won't affect any of
the newer formats, since they don't use the 'omitted' construct in the same way.

Since this heuristic was put into patch at a time when email / etc ate trailing
white space on a regular basis, and since it's clear that this heuristic is the
wrong thing to do at least some of the time, it's better to remove it
entirely. It's not been needed for maybe 20 years since patch files are not
usually corrupted. If there are a small number of patch files that would benefit
from this corruption fixing, those already-currupt patches can be fixed by the
addition of blank lines. I'd wager that no one will ever come to me with an
example of a once-working patch file that breaks with this change. However, I
have 2 patches from the first 195 patches to 2.11BSD that are affected by this
bug, suggesting that the relative frequency of the issue has changed
signficantly since the original heuristic was put into place.

Reviewed by: phk@
Differential Revision: https://reviews.freebsd.org/D26081

show more ...

patch(1): give /dev/null patches special treatment

We have a bad habit of duplicating contents of files that are sourced from
/dev/null and applied more than once... take the more sane (in most ways

patch(1): give /dev/null patches special treatment

We have a bad habit of duplicating contents of files that are sourced from
/dev/null and applied more than once... take the more sane (in most ways)
GNU route and complain if the file exists and offer reversal options.

This still falls short a little bit as selecting "don't reverse, apply
anyway" will still give you duplicated file contents. There's probably other
issues as well, but awareness is the first step to happiness.

MFC after: 2 weeks
Differential Revision: https://reviews.freebsd.org/D21535

show more ...

MFHead@r345275