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

Reply via email to