Page MenuHomeFreeBSD

Add Caveats Section to fgetln(3)
ClosedPublic

Authored by gbe on May 19 2020, 1:53 PM.

Details

Summary

Add a Caveats Section to fgetln(3)

Obtained from: OpenBSD

Test Plan

mandoc output review

Diff Detail

Repository
rS FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

gbe requested review of this revision.May 19 2020, 1:53 PM
gbe created this revision.
yuripv added inline comments.
lib/libc/stdio/fgetln.3
142 ↗(On Diff #71991)

Nitpicking: != NULL, per style(9), "Test pointers against NULL"

Although there is nothing wrong with this per se, applications can deal with this problem more easily using getline(3) (which is also standard).

Although there is nothing wrong with this per se, applications can deal with this problem more easily using getline(3) (which is also standard).

We could mention this (the reference to getline(3)) in that section.

lib/libc/stdio/fgetln.3
142 ↗(On Diff #71991)

Did you mean something like the following for the while condition?

((buf = fgetln(fp, &len)) != null)

lib/libc/stdio/fgetln.3
142 ↗(On Diff #71991)

Exactly, but as it's really nitpicking, feel free to ignore this comment.

  • Add NULL pointer check for the while condition

Although there is nothing wrong with this per se, applications can deal with this problem more easily using getline(3) (which is also standard).

We already .Xr getline(3). Would that be enough, or should we explicitly reference it as a better alternative in the CAVEATS section?

This revision is now accepted and ready to land.Jun 14 2020, 9:35 AM
This revision was automatically updated to reflect the committed changes.
0mp added inline comments.
head/lib/libc/stdio/fgetln.3
130

I am pretty sure that you're going to hear voices on the mailing list saying that you should break the line after the comma in this case. ;)

134

s/won't/will not/

135

I'd put "a" into the next line.

149

Should we check the output of memcpy?

head/lib/libc/stdio/fgetln.3
132

Also, as per style.mdoc(5), Dq should be used here instead of Sq.

What do you think, @bcr?

@0mp I see that you have put a lot of comments in already approved and committed differential. Could provide just a new differential with you comments applied to manpage? I wasn't aware of style.mdoc(5), definitely read it this afternoon.

You can always re-open the revision.
@0mp: Sure, Dq is better. Sorry I missed that, causing extra work.

In D24916#558726, @bcr wrote:

You can always re-open the revision.
@0mp: Sure, Dq is better. Sorry I missed that, causing extra work.

In theory a differential can be re-opened, but dealing with an already committed one puts you in the position to personally editing the resulting diff by hand, which is, to say at least, frustrating.

In D24916#558733, @gbe wrote:
In D24916#558726, @bcr wrote:

You can always re-open the revision.
@0mp: Sure, Dq is better. Sorry I missed that, causing extra work.

In theory a differential can be re-opened, but dealing with an already committed one puts you in the position to personally editing the resulting diff by hand, which is, to say at least, frustrating.

Haha, obviously I've not thought for a second that you'd need to edit those diffs by hand :D That would be cruel.

I wasn't aware that this revision had been already committed when I submitted my comments. Sorry about that. If you decide to fix address those comments in another differential revision I'll be happy to review them again. When it comes to memcpy though, I'd ask a src committer for an opinion.