Re: [PATCH]proposal for a fake_strore_url helper wrapping.

On 09/11/2012 01:52 AM, Amos Jeffries wrote:
>
> If by this you mean the helper/ section fake helper. The
> helper/url_rewrite/fake should be sufficient. url-rewrite, store-url,
> and location-rewrite (also not ported) all have the same API
> limitations and format so the helpers can all be in
> helper/url_rewrite/ together.
This is the first time I have heard about location-rewrite and it's a
nice and interesting interface.
it can be helpful for cdn stuff if i got it right.
But this is not for now.
if and after I will finish this project I might take a look at this.
>
>
>> this part is pretty important by itself because no one even done that
>> in the 3+.
>> 2. embed the code that actually handles the data from the external
>> program when storing it in squid process.
>> 3. use it at every point needed in squid while handling the requests.
>>
>>
>>
>>
>>>
>>> Please do not forget to document all new members (using doxygen
>>> comments) -- this is one of the mandatory requirements for patches.
>>> And,
>>> as Amos pointed out, remove "for storeurl" marks -- everything you
>>> change in this patch is for the storeurl feature (or at least it should
>>> be) so the diff blobs themselves can be used as such a mark.
>> Since I Have never used doxygen and friends in my code before I will
>> try to learn them on the way.
>
> Basically C/C++ comments, just add an extra / or * ( "///..." or
> "/** ... */") to start the comment and ensure clear paragraph separation.
>
> For functions/methods with return values or special mention required
> on parameters use \param and \return or \retval. You can see examples
> of these around the code or we can assist with corrections during audit.
>
seems like pretty simple to get.
>>
>>>
>>> If this code is not sufficient to support the storeurl feature, I do
>>> not
>>> think it should be committed to trunk -- it has no stand-alone value.
>>> Commit it to your own branch so that you can maintain the progress
>>> history and post incremental patches for discussion. When the
>>> feature is
>>> completed, your branch can then be merged into trunk.
>> I do have a plan since From all the data I gathered and code review I
>> did until now it's pretty straight forward.
>>
>> I will look at the guidelines of squid3 code (started already)
>>
>>
>> So you do remind me that I didnt proposed *yet* the basic idea which
>> is critical.
>> what I wanted is to get all the patches ready and then commit them.
>> I must state I'm testing my code and not waiting until it breaks
>> something.
>>
>> If you do want to know exactly what is being done or what I propose:
>> get the rewritten url from the external software.
>> put it in a char * variable (while NULLED at initialization).
>
> I've been wondering if it has to be a char* for any particular reason
> (likely deep in store code). If not please use String type provided by
> Squid which will manage all the allocation/deallocation complexity for
> you.
>
It dosnt need to be a char* but this how it's been done before.
If there is something that forces me to use it I think it's other
variables on the map.
Many of them are char* or const char* but I suppose a simple conversion
method is there.
I have seen the String type on the map there so I believe It will be
simple for me to use.

Update: I'm reviewer every point where mem_obj->url is used and I was
thinking that using an interface such that was mentioned
before(originalUrl,storeUrl) will be a stater.
Instead of changing the object by itself I will create a method the will
get it from mem_obj->url
And will create another one for storeUrl.
an example method i saw is:
void
MemObject::resetUrls(char const *aUrl, char const *aLog_url)

so:
char const
MemObject::storeUrl()

char const
MemObject::originalUrl()

should be ok?

I want to use this in the transition so I can easily follow where and
what is being done using a debugs section with HERE and friends.

It will be very easy to reactor temporarily That way for me since one
time run with debug section can revile Many things on the process that I
might miss by eye.

While looking at the MD5 debug I have seen stuff happens like really
unexplained to me yet that i'm looking for answers in the code.
I have seen that there is a point in runtime the requests calculated a
MD5 hash which is the same every time.
later I saw MD5 that changes every time which I assume is more for the
whole object\request.

I hope today I will finish re-factoring the usage from ->url to
->origianlUrl.

> <SNIP>
>
> Amos

Thanks for everything,
Eliezer
Received on Tue Sep 11 2012 - 07:08:31 MDT