Page MenuHomeFreeBSD
Differential D2830

Fix empty label handling for cd9660.
ClosedPublic
Actions

Authored by trasz on Jun 15 2015, 6:44 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, May 14, 1:52 PM
Unknown Object (File)
Sat, May 11, 11:30 PM
Unknown Object (File)
Sat, May 11, 1:51 AM
Unknown Object (File)
Fri, May 10, 12:40 PM
Unknown Object (File)
Fri, May 10, 12:35 PM
Unknown Object (File)
Fri, May 10, 12:32 PM
Unknown Object (File)
Mon, Apr 29, 4:53 PM
Unknown Object (File)
Mon, Apr 29, 4:50 PM
View All 90 Files
Subscribers
imp

Details

Reviewers
emaste
asomers
Commits
rS284582: Fix off-by-one error in fstyp(8) and geom_label(4) that made them use
Summary

Without this fix, empty label is being incorrectly reported as a one
character string, " ".

Similar fix might be needed for geom_label(4) module this code come
from.

PR: 200828

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

trasz updated this revision to Diff 6218.Jun 15 2015, 6:44 PM
trasz retitled this revision from to Fix empty label handling for cd9660. Without this fix, empty label is being incorrectly reported as a one character string, " ". Similar fix might be needed for geom_label(4) module this code come from. PR: 200828.
trasz updated this object.
trasz edited the test plan for this revision. (Show Details)
Herald added a subscriber: imp. · View Herald TranscriptJun 15 2015, 6:44 PM
trasz added reviewers: emaste, asomers.Jun 15 2015, 6:45 PM
emaste edited edge metadata.Jun 15 2015, 7:13 PM

Looks OK to me although I'd have a small preference for changing this to avoid the signed i.

trasz updated this revision to Diff 6235.Jun 16 2015, 1:27 PM
trasz retitled this revision from Fix empty label handling for cd9660. Without this fix, empty label is being incorrectly reported as a one character string, " ". Similar fix might be needed for geom_label(4) module this code come from. PR: 200828 to Fix empty label handling for cd9660..
trasz edited edge metadata.
trasz updated this object.

Deduplicate the code and add the same fix for geom_label(4).

emaste added inline comments.Jun 16 2015, 2:33 PM
usr.sbin/fstyp/fstyp.c
108–109 ↗(On Diff #6235)

Perhaps call this rtrim instead. There's a precedent in various languages for ltrim/rtrim/trim, with trim removing both leading and trailing space.

trasz updated this revision to Diff 6237.Jun 16 2015, 4:09 PM

s/trim/rtrim/

asomers accepted this revision.Jun 16 2015, 4:14 PM
asomers edited edge metadata.

I agree with emaste's comment. Other than that, LGTM.

This revision is now accepted and ready to land.Jun 16 2015, 4:14 PM
trasz added a comment.Jun 17 2015, 8:39 AM

Ed, does it look ok? If so, can you mark it Accepted, so you end up in "Reviewed by"? Thanks!

emaste accepted this revision.Jun 17 2015, 12:54 PM
emaste edited edge metadata.
asomers mentioned this in D2800: Bug 200828 - fstyp(8) prints " " as the label of a labelless cd9660 filesystem.Jun 18 2015, 3:23 PM
Closed by commit rS284582: Fix off-by-one error in fstyp(8) and geom_label(4) that made them use (authored by trasz). · Explain WhyJun 18 2015, 9:56 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathSize
head/
sys/
geom/
label/
2 lines
15 lines
10 lines
10 lines
usr.sbin/
fstyp/
10 lines
1 line
16 lines
10 lines

Diff 6311

View Options

head/sys/geom/label/g_label.h

Loading...
View Options

head/sys/geom/label/g_label.c

Loading...
View Options

head/sys/geom/label/g_label_iso9660.c

Loading...
View Options

head/sys/geom/label/g_label_msdosfs.c

Loading...
View Options

head/usr.sbin/fstyp/cd9660.c

Loading...
View Options

head/usr.sbin/fstyp/fstyp.h

Loading...
View Options

head/usr.sbin/fstyp/fstyp.c

Loading...
View Options

head/usr.sbin/fstyp/msdosfs.c

Loading...