I am committing a version 3 of the patch.

Amos Jeffries wrote:
Tsantilas Christos wrote:
Alex Rousskov wrote:
On 07/09/2009 12:06 AM, Amos Jeffries wrote:
Tsantilas Christos wrote:


 **  Please make fqdncache_entry / ipcache_entry public classes instead
of functional structs. Some gcc complain and doxygen won't document
their functions properly.

Sorry, I am not sure what you mean. What is a "functional struct"? Do
you want us to replace "struct fqdncache_entry" with "class
fqdncache_entry" and add "public:"?

No objections, of course, just wanted to minimize merging headaches by
keeping out-of-scope changes to the minimum...

I did not touch it. The struct used in many places inside squid3


Please do change.

The rest of the code fails to prefix the names with "struct" the typedef made that possible.

Using class and public: is the C++ upgrade equivalent to the typedef and allows functions/methods inside the struct where I suspect the typedef did not.

If there are code references to "struct fqdncache_entry" or "struct ipcache_entry", they are well within scope of the change removing relevant typedefs.
FWIW: I find none in the existing code. Which makes the change safe.


OK done.


>..............

My compiler (because it is very clever @#%$##$%#!) does not allow me to remove the LFT_HTTP_SENT_STATUS_CODE_OLD_30 from switch statements.

Is that because of a lack of default: ?
Yes, but it is not good idea to add a "default:" statement here.


Never mind then. Just add a note saying what compiler complains if its removed so I don't get just as smart and do it :)

OK I add the comment.



I spy a goto that can die...
     case CLF_NONE:
             goto last;
@@ -1458,7 +1863,15 @@
 last:
     (void)0; /* NULL statement for label */
+}

==> equates to "return;". Please check for others in that same function.

I prefer to not touch it at this time. We can change it in trunk with a separate patch.

Okay. Fair enough.


Amos

Attachment: 3p1-plus-mod-v3.patch.gz
Description: Unix tar archive

Reply via email to