Andrew Stitcher escribió:
On Tue, 2009-03-03 at 17:10 +0100, Manuel Teira Paz wrote:
Coming back to this issue.

I don't like the option of having a different Socket.h for windows and posix, since they would only differ in one include sentence.

On the other hand, I don't see any problem into having socklen_t defined into IntegerTypes.h, since that's what it is: an integer type. Perhaps including the whole <sys/socket.h> in the posix IntegerTypes.h is not very polite, but, what other options do we have?

To have a specific SocketTypes.h ? It seems a little odd, just to support socklen_t. Or to define a own typedef to be used in the Socket::accept method (the only one using socklen_t actually)?

So, my +1 for adding sys/socket.h to the posix IntegerTypes.h header.

I think this is the wrong solution: If socklen_t isn't a platform
portable type then it has no place being used in platform independent
header - The only platform dependent code should be in subdirectories of
qpid/sys, not in qpid/sys itself. So we should change the type to be a
different type that is platform portable and deal with the platform
specifics in the platform layer instead (simple casting will do).

I suggest we just change qpid/sys/Socket.h to use uint32_t instead - in
fact I will make that change unless anyone complains by the end of day.
uint32_t is big enough for any currently conceivable socket address for
any currently conceivable protocol (dons hard hat and ducks).

My fear here is that since POSIX.g seems to have defined socklen_t as size_t (a wrong decision, anyway), a 64 bits platform can handle the passed socklen_t pointer as something handling 8 bytes. Something like bzero(ptr, sizeof(ptr)) when ptr is a socklen_t pointer, will lead to overwriting past the end of our uint32_t pointer. So, perhaps casting is not enough in this case.

Reading the socket.h solaris header, it seems that XPG5 (Unix98?) defines socklen_t as uint32_t, but XPG4 defines it as size_t.

About the decision itself, I think it is the right one, and socklen_t shouldn't appear in a sys top header.


Regards.


Andrew



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org



---------------------------------------------------------------------
Apache Qpid - AMQP Messaging Implementation
Project:      http://qpid.apache.org
Use/Interact: mailto:dev-subscr...@qpid.apache.org

Reply via email to