You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
338 lines
13 KiB
338 lines
13 KiB
This is a list of code "concerns". these have been removed from the code and
|
|
placed here so that the code will not contain untoward comments.
|
|
|
|
rx\read.c and write.c
|
|
tomm claims that the memory manager has been fixed before nt4.0 shipped
|
|
so that it is no longer necessary to synchronize flushers by acquiring and
|
|
releasing the pagingio resource. the is in the check for noncachedio when there
|
|
is a private cachemap. we should make sure that everyone else removes their
|
|
synchronization or we should put ours back.
|
|
|
|
rx\read.c and write.c
|
|
the pipe read/write paths should be completely separated in the wrapper
|
|
and the opportunity to do so should be presented in the mini.
|
|
|
|
rx\read.c
|
|
there is a place where we set status pipe empty; at that point we should
|
|
also backoff a la rdr1
|
|
|
|
rx\lock.c
|
|
there should be backoff logic
|
|
|
|
rx\strtstop.c
|
|
the call to rxforcenettablefinalization should be per-minirdr and should
|
|
be moved out of the iftest.
|
|
|
|
rx\strtstop
|
|
acquireing the netname table lock is was only required becauseof
|
|
precious servers. now that that's gone. we should remove the lock. but, it's
|
|
safe, of course.
|
|
|
|
mrx in general
|
|
we should review the throttling constants....seem pretty conservative to
|
|
me; we should consider being substantially more aggressive.
|
|
|
|
mrx
|
|
we should have precious server code.
|
|
|
|
rx\lockctrl.c
|
|
we should only allocate a lock structure if someone does a lock! same
|
|
with the oplock structure. ntfs does this. obviously, we would have a change in
|
|
fcb.h to go with this.
|
|
|
|
rx\lockctrl.c
|
|
the last little bit of getting the refcounts right was to account for
|
|
the fact that the fspdispatch does not remove a reference if pending is
|
|
returned. this is probably wrong....and we should map over to a scheme where the
|
|
fsd and fsp dispatches are the ones that do this instead of having every common
|
|
routine do it. this would be a major change but it would really make things more
|
|
consistent.
|
|
|
|
rx\lockctrl.c
|
|
it is stupid that we will have to lookup the lock again if it fails
|
|
(async completion path). we should ask for an fsrtl interface that removes a
|
|
lock that i provide.
|
|
|
|
rx\create.c
|
|
in RxCreateFromNetRoot, after the call to
|
|
|
|
rx\ntinit.c
|
|
in RxGetRegistryParameters, we should allocate space for Storage
|
|
instead. really what we should do is to implement and export a routine that both
|
|
allocates and reads a string........for Ulong no allocation should be necessary
|
|
|
|
rx\ntfsp.c
|
|
in fspdispatch and also addtoworkq, we use the wrapper's device object
|
|
for throttling instead of the minirdr's device object. this will change.
|
|
|
|
rx\ntfsp.c
|
|
in fspdispatch, if status_retry is returned then we will have already
|
|
completed the packet.
|
|
|
|
rx\ntfsp.c
|
|
prepostirp should be made pageable; also, i'm not too happy
|
|
|
|
rx\ntfsd.c
|
|
initialization of prefixtable should be in RxInitializeDispatchVectors
|
|
|
|
rx\ntfsd.c
|
|
RxFsdCommonDispatch in an excellent canididate for inlining
|
|
|
|
rx\ntfsd.c
|
|
we shouldn't be checking everywhere for null dispatch points....rather
|
|
we should fill them in with a good default
|
|
|
|
rx\ntfsd.c
|
|
we should remove the async code from everywhere else and have it only in
|
|
fsddispatch and fspdispatch. same with retry. this would get rid of the
|
|
"should i remove a reference or not" decision.
|
|
|
|
rx\??
|
|
the code that chains together pagingio reads should be removed
|
|
|
|
rx\ntfastio.c
|
|
in fastiocheckifpossible...return the error directly. also, we should
|
|
the cached netroottype as well as nodetype to determine if it's a
|
|
diskfile. also in markoncleanup.
|
|
|
|
rx\ntexcept.c
|
|
i removed this from the header
|
|
|
|
|
|
Original Comments
|
|
This routine process an exception. It either completes the request
|
|
with the saved exception status or it sends it off to IoRaiseHardError()
|
|
|
|
My New Improved Version
|
|
This routine processes an exception from the top level FSD routine. You can tell because it has
|
|
an Irp w/o an RxContext. The original code did all kinds of stuff like raising hard errors and
|
|
posting and such. My take is different...if you get here, things are hosed. So we either complete
|
|
or we bugcheck. PERIOD. we can always be more liberal later...........
|
|
|
|
rx\ntfsd.c
|
|
in RxDevFcbQueryDeviceInfo, the special handling for pipes should be
|
|
removed.
|
|
|
|
rx\lockctrl.c
|
|
don't allocate a lock structure unless you need one. also in fcb.h and
|
|
fcbstruc.c
|
|
|
|
rx\lockctrl.c
|
|
at rxunlock, we could be called from the cleanup routine on the last
|
|
handle. in such a case, we should just swallow the unlocks. this might be a
|
|
minirdr thing instead.
|
|
|
|
rx\fsctrl.c
|
|
in RxLowIoFsCtlShell, we should have to switch out again. we should have
|
|
handled the peeknamepipe throttling directly in the entry routine.
|
|
|
|
rx\flush.c
|
|
maybe we should try to route for filetypes that we don't understand. or
|
|
at least to have the minirdr signal that we should doit thru some stateful call.
|
|
|
|
rx\fileinfo.c
|
|
i remove the following
|
|
//CODE.IMPROVEMENT having a separate wrapper for every routine is stupid. we should (a) only
|
|
// have it if we need it and (b) preinitialize the buffer and lengthremaining so we don't always
|
|
// have to pass them.
|
|
|
|
[considering this we should fix the trace as well........]
|
|
|
|
and
|
|
//CODE.IMPROVEMENT probably there's a better way than this to figure out whether
|
|
// we need the resource at all or need it shared or need it excl.
|
|
// for name, we don't need it at all since the name never changes
|
|
|
|
and from allinfo handling
|
|
//CODE.IMPROVEMENT.ASHAMED either here or in the minirdr we should glob up call so that multiple trips
|
|
// are not taken
|
|
|
|
rx\fileinfo.c
|
|
you shouldn't do the fsrtloplock check for pipes. this keeps us from not
|
|
allocating the struct
|
|
|
|
rx\fileinfo.c
|
|
should we really change the name in the fcb after a rename. it
|
|
complicates locking a bit for querys
|
|
|
|
rx\dirctrl.c
|
|
no need to allocate templace buffer if it's small enough. most templates
|
|
are probably less than 8.3 in size
|
|
|
|
rx\create.c
|
|
the writeserialization field in the fobx is used by spoolfiles as well
|
|
as pipes. maybe a name change to the specific field could reflect this dual
|
|
usage.
|
|
|
|
rx\create.c
|
|
perhaps we should wait to finalize on a failed open.
|
|
|
|
rx\create.c
|
|
it would be good to "summarize" the fcb names that are logged
|
|
|
|
rx\create.c
|
|
we should have a flag on fcb/netroot somewhere to denote that only
|
|
32-bit operations are possible instead of having every mini have to check.
|
|
|
|
rxce\scavengr.c
|
|
in scavenge-fobxs, starting at the beginning of the list each time is
|
|
bad. what if there is some problem then we'd never get off the first guy.
|
|
|
|
rxce\prefix.c
|
|
i removed this comment from the lookup
|
|
// CODE.IMPROVEMENT we need code to lookup vnetroots MUCH faster....like a 26way cache........
|
|
// failing that, we could at least not do the lookups (and hash computation)
|
|
// until we're on the third component, i.e. \;m:\server\share. the cache is better.
|
|
|
|
rxce\fcbstruc.c
|
|
in finishfcbinitialization, we should not initialize the lock structure
|
|
for pipes. of course, we shouldn't even have a lock structure yet but that's a
|
|
different c.i.
|
|
|
|
rxce\buffring.c
|
|
i removed the following comment from RxChangeBufferingState
|
|
//CODE.IMPROVEMENT we sure use up a lot of log entries in here! maybe we should put this state info
|
|
// on the fcb.
|
|
|
|
BUGBUG
|
|
|
|
rxce\rxcontx.c
|
|
the priority boost is set to disk....probably should be net. actually
|
|
should be mini parameter.
|
|
in the place where we check for advance-only on setfileinfo, we'd need a
|
|
bit different for locals. there are a couple of places.
|
|
|
|
rxce\rxconnct.c
|
|
i removed the following from RxConstructSrvCall
|
|
//BUGBUG we should have some sort of refcount machanism to prevent a minirdr from
|
|
// unregistering while he's part of a calldown
|
|
//also, we shouldn't necessarily be calling down if this guy is not a UNC provider
|
|
|
|
rxce\fcbstruc.c
|
|
in RxWaitForStableCondition, you need to be able to cancel.
|
|
|
|
rxce\fcbstruc.c
|
|
in RxCreateNetRoot,
|
|
//TRUE means case insensitive compares BUGBUG get this from srvcall
|
|
|
|
turns out, the whole issue of casing is incorrectly handled because
|
|
there is no way to get casing handled on a per-call basis in case the
|
|
casesensitive flag is set on open.
|
|
|
|
rxce\fcbstruc.c
|
|
in RxCreateNetFcb,
|
|
//BUGBUG: this should be on the fobx......not on the FCB. it should then be used correctly
|
|
// in the querynameinfo code. same with stripped backslash
|
|
the flag in question is RX_CONTEXT_CREATE_FLAG_ADDEDBACKSLASH.
|
|
|
|
rx\strtstop.c
|
|
in RxStopMinirdr, the comment was removed from the following line
|
|
RxTerminateScavenging(RxContext); //BUGBUG separate into the two parts and make the purge depend on the DObj
|
|
|
|
rx\lockctrl.c
|
|
in RxUnlockOperation, the following comment was removed where we try to
|
|
allocate
|
|
//bugbug we cannot fail........i guess what we'd have to do is to send the unlock
|
|
//from here!
|
|
|
|
rx\fileinfo.c
|
|
in RxQueryStandardInfo, i removed this about the fact that links are
|
|
returned incorrectly
|
|
//BUGBUG there is a problem here with number of links...basically, the deal is that numberoflinks is
|
|
//dynamic whereas everything else in the structure is knowable by the client side under oplock.
|
|
|
|
rx\create.c
|
|
in , i removed this about downlevel canonicalization.
|
|
DOWNLEVEL.BUGBUG we don't really do it yet but we do
|
|
componentize and check for .. and .; for names that come down from win32
|
|
this is fine.....for direct names going to downlevel servers we should do
|
|
more checking maybe.
|
|
|
|
rx\create.c
|
|
in RxCanonicalizeFileNameByServerSpecs, i removed the comment on this
|
|
line both places
|
|
&& RX2C_IS_DOT(Buffer[firstchar+2])) //??BUGBUG what about a alternate stream :.
|
|
|
|
rx\read.c
|
|
need to implement a policy for controlling readahead differently for
|
|
different customers
|
|
|
|
rx\dirctrl.c
|
|
i removed this at the place when the uniarg template is comared
|
|
// then match all names. joejoe....do i really need all this?? ntfs doesn't!
|
|
|
|
rx\create.c
|
|
this from the header comments in RxCreateFromNetRoot:
|
|
joejoe There is some possibility that we should just keep shared on the tablelock until we
|
|
dispatch to the minirdr. That would save one release/acquire per open.
|
|
|
|
rx\cleanup.c
|
|
removed the joejoe from this
|
|
if (NeedDelete || NeedPurge) {
|
|
//joejoe need to put in logic to keep a new read/write from
|
|
//reinitializing
|
|
RxDbgTrace(0, Dbg, ("CleanupPurge:MmFlushImage\n", 0));
|
|
MmFlushImageSection(&capFcb->NonPaged->SectionObjectPointers,
|
|
MmFlushForWrite);
|
|
|
|
rx\cleanup.c
|
|
and from this.....
|
|
KeQuerySystemTime( &CurrentTime );
|
|
|
|
//
|
|
// Note that we HAVE to use BooleanFlagOn() here because
|
|
// FO_FILE_SIZE_CHANGED > 0x80 (i.e., not in the first byte).
|
|
//
|
|
|
|
//joejoe we need to probably do changetime independently
|
|
//perhaps the smartest thing would just to put a basicinfo
|
|
//struc in the fcb
|
|
|
|
rxce\rxconnct.c
|
|
from RxConstructSrvCall, removed the joejoe
|
|
//joejoe we should have a structure tag on this
|
|
pSrvCalldownStructure = RxAllocatePoolWithTag(
|
|
NonPagedPool,
|
|
sizeof(MRX_SRVCALLDOWN_STRUCTURE) +
|
|
(sizeof(MRX_SRVCALL_CALLBACK_CONTEXT) * NumberOfMinirdrs),
|
|
'CSxR' );
|
|
if (pSrvCalldownStructure == NULL) {
|
|
ExReleaseFastMutexUnsafe(&RxData.MinirdrRegistrationMutex);
|
|
return STATUS_INSUFFICIENT_RESOURCES;
|
|
}
|
|
|
|
rxce\fcbstruc.c
|
|
from RxFinalizeSrvOpen, removed the joejoe remark
|
|
if (ThisSrvOpen->Condition == Condition_Good) {
|
|
RxPurgeChangeBufferingStateRequestsForSrvOpen(ThisSrvOpen);
|
|
}
|
|
|
|
//joejoe we could put sme more asserts in here about empty srvfobxlist etc.
|
|
|
|
// close the file.
|
|
MINIRDR_CALL_THROUGH(Status,Fcb->MRxDispatch,MRxForceClosed,((PMRX_SRV_OPEN)ThisSrvOpen));
|
|
}
|
|
|
|
rxce\buffring.c
|
|
from RxChangeBufferingState, removed joejoe
|
|
MINIRDR_CALL_THROUGH(
|
|
Status,
|
|
Fcb->MRxDispatch,
|
|
MRxCompleteBufferingStateChangeRequest,
|
|
(RxContext,(PMRX_SRV_OPEN)SrvOpen,Context));
|
|
}
|
|
|
|
//joejoe this could fail!!!! we should place the fcb into a
|
|
// "failed oplock break" state.
|
|
|
|
RxDereferenceAndDeleteRxContext(RxContext);
|
|
}
|
|
|
|
AND
|
|
|
|
} finally {
|
|
ClearFlag(Fcb->FcbState,FCB_STATE_BUFFERSTATE_CHANGING); //this is informational for error recovery
|
|
//joejoe there should be a callout here for a readpolicy...not vanilla..........
|
|
}
|
|
|
|
|