If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Questionable additional code

VERIFIED FIXED in mozilla0.9.4

Status

()

Core
Networking: FTP
P4
normal
VERIFIED FIXED
17 years ago
16 years ago

People

(Reporter: Ulrich Drepper, Assigned: dougt)

Tracking

({footprint})

Trunk
mozilla0.9.4
footprint
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: review submitted patch.)

Attachments

(1 attachment)

(Reporter)

Description

17 years ago
I'm currently reviewing various pieces of code here and there and came across
netwerk/streamconv/converters/nsFTPDirListingConv.cpp.  The function
nsFTPDirListingConv::MonthNumber contains some code to parse the month.

This code is questionable as is.  Some code paths are not doing any checks
which are not necessary.  E.g., if the strings starts with an eff February
is assumed.  I agree with this.  But in some code paths tests are performed
which do not help to discriminate different valid results.  E.g., if the first
character is m or M a test is performed whether an a or A follows.  But this
'if' has no 'else' path and is therefore unnecessary.

I'll append shortly a patch which removes the unnecessary tests.  This reduces
the code size for me by 140 bytes (hey, it's more than 1% :-).
(Reporter)

Comment 1

17 years ago
Created attachment 38906 [details] [diff] [review]
Patch to remove unnecessary tests
(Assignee)

Comment 2

17 years ago
didn't write the code, but I will try to review the patch for 0.9.2.
Whiteboard: review submitted patch.
Target Milestone: --- → mozilla0.9.2
(Reporter)

Updated

17 years ago
Keywords: footprint, patch
Hardware: PC → All
(Assignee)

Updated

17 years ago
Priority: -- → P4
(Assignee)

Updated

17 years ago
Target Milestone: mozilla0.9.2 → mozilla0.9.3
(Assignee)

Comment 3

16 years ago
bulk move to 0.9.4
Target Milestone: mozilla0.9.3 → mozilla0.9.4
(Assignee)

Comment 4

16 years ago
fix checked in

Checking in nsFTPDirListingConv.cpp;
/cvsroot/mozilla/netwerk/streamconv/converters/nsFTPDirListingConv.cpp,v  <-- 
nsFTPDirListingConv.cpp
new revision: 1.54; previous revision: 1.53
done
Checking in nsFTPDirListingConv.h;
/cvsroot/mozilla/netwerk/streamconv/converters/nsFTPDirListingConv.h,v  <-- 
nsFTPDirListingConv.h
new revision: 1.14; previous revision: 1.13
Status: NEW → RESOLVED
Last Resolved: 16 years ago
Resolution: --- → FIXED

Updated

16 years ago
QA Contact: tever → benc

Comment 5

16 years ago
verified. patch checked in.

Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.