On 4/01/2013 7:10 a.m., Eliezer Croitoru wrote:
Thanks Alex,
On 1/3/2013 6:23 PM, Alex Rousskov wrote:
One of the reasons is that sooner or later the current or updated code
will crash while "accessing the request in these situations". Just
because you have carefully identified when the request should or should
not be used now, does not mean that your identification will remain
valid a year from now or that nobody will copy your code to a different
location where it will break next week.
Well I can't be responsible for the wind in Taiwan while I'm here.
Sorry.(there is more to come..)
If there will be a change with such a huge impact a basic runtime test
should find that so allow me to doubt a bit.
This has nothing to do with object orientedness. This is about giving a
correct way of accessing information that may be present in two
different places, one of which may be invalid. If testing request for
being nil is sufficient, this is a simple change. If it is not
sufficient because request may be non-nil but invalid, then it is
probably too dangerous to commit this code and more work is needed to
make it reliable (which may include changing other code, of course).
OK got you now.
I need to rephrase something:
There are places in Squid code (not related to my code) which the code
scope allows access to an invalid request.(it's like saying a kid poops)
Leave these aside and while researching the code my main effort was to
back-trace couple functions from runtime back to the request creation.
This research provided me with enough knowledge about the code which
makes this patch code Stable in many aspects.
One of these aspects is not accessing(r\w) what and when not needed at
all.
I choose by hand with a needle the specific points of code which sets
and gets data in the main components which affect the identifiers of
the requests while considering the scopes of the code carefully.
Sure, but you coded in that patch a whole bunch of fetch rules which all
said: if a request was present, use its storeId, if none was present
use the alternative variable storeId.
All I was asking for was to take that piece of logic decision which was
repeated in a handful of places (with slightly different boolean tests)
and make a inline accessor function which anybody can call without
needing to reasearch teh diferentce between the HttpReqeust:storeId and
the alternative one, or to re-code the test logic themselves. We should
be able to just call storeId() accessor from the state object and get
whatever the current storeId is. No complex conditionals.
Amos
