bugCERTI - Bugs: bug #24537, Remove unused Socket::getClass()?

 
 

bug #24537: Remove unused Socket::getClass()?

Submitted by:  Petr Gotthard <gotthardp>
Submitted on:  Sun 12 Oct 2008 12:18:57 PM UTC  
 
Category: CERTISeverity: 2 - Minor
Item Group: Software errorStatus: Ready For Test
Privacy: PublicAssigned to: Petr Gotthard <gotthardp>
Open/Closed: ClosedRelease: CVS
Reproducibility: NoneFixed Release: 3.3.2

Add a New Comment(Rich Markup)
   

You are not logged in

Please log in, so followups can be emailed to you.

 

Mon 13 Oct 2008 11:29:48 AM UTC, comment #5:

Done. Bug #24545 will work on further enhancements.

Petr Gotthard <gotthardp>
Project MemberIn charge of this item.
Mon 13 Oct 2008 11:20:52 AM UTC, comment #4:

Dynamic cast seems to be the right short-term solution.
But the long-terme one would be to have a way to
(polymorphically) check for a particular functionnality.

In the current case we have to cast in order to
get the getPeerName method which is available
in SecureTCPSocket but is not in SocketTCP.

For the long-term solution we may continue to use
dynamic_cast if we find a "clean" class hierarchy
or we may use a pattern like "visitor" in order
to delegate the canFederateAccessData to the
socket class.

Eric NOULARD <erk>
Project Administrator
Mon 13 Oct 2008 11:12:44 AM UTC, comment #3:

Yes, it's perfectly OK. Thanks. Just a question:

Is dynamic_cast the right solution? Is the method you mentioned used to "check socket capabilities", or to verify that a particular class is used? I mean, can there can be other class providing the same functionality (e.g. OpenSSL instead of GSSAPI)?

Petr Gotthard <gotthardp>
Project MemberIn charge of this item.
Mon 13 Oct 2008 10:12:30 AM UTC, comment #2:

Petr,

I did do a first pass fix for this one.
And I did open another bug for handling it
in a more elegant way for the long-term.
https://savannah.nongnu.org/bugs/index.php?24545

Now, you should be able to remove getClass without pain.
I'll do the work for a clean Socket hierarchy later,
if that's OK for you?

Eric NOULARD <erk>
Project Administrator
Mon 13 Oct 2008 09:35:08 AM UTC, comment #1:

In fact it IS USED.
See in:
bool
libCERTI/SecurityServer.cc:SecurityServer::canFederateAccessData

you have

however I totally agree with you this is not
nice C++ design.
We may faithfully replace this with a dynamic_cast.

I did give it a try but end up with unexpected result
because there are here and there (C-style) static cast
to SecureTCPSocket for socket which are not !!

I may handle this one but not today because it may
be more "difficult" than it seems...

Eric NOULARD <erk>
Project Administrator
Sun 12 Oct 2008 12:18:57 PM UTC, original submission:

There is an unused function Socket::getClass(). I think in this case the type information is unnecessary and this function should be removed. It forces us to "register" child classes in Socket.hh, which prevents us from building communication plug-ins. What do you think?

Attached patch is an overview of necessary changes.

Petr Gotthard <gotthardp>
Project MemberIn charge of this item.

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach File(s):
   
   
Comment:
   

Attached Files
file #16658:  getclass.patch added by gotthardp (2KiB - text/x-patch - Proposed change.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by erk (Posted a comment)
  • -unavailable- added by gotthardp (Submitted the item)
  •  

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Wed 29 Apr 2009 09:13:07 AM UTCerkOpen/ClosedOpen=>Closed
      Fixed ReleaseNextRelease=>3.3.2
    Mon 13 Oct 2008 11:29:48 AM UTCgotthardpStatusNone=>Ready For Test
    Sun 12 Oct 2008 12:18:57 PM UTCgotthardpAttached File-=>Added getclass.patch, #16658

    Back to the top


    Powered by Savane 3.1-cleanup1