On Tue, 2011-11-29 at 03:44 +0100, Jörn Engel wrote:
> On Mon, 28 November 2011 17:13:16 -0800, Nicholas A. Bellinger wrote:
> > On Mon, 2011-11-28 at 01:05 -0800, Nicholas A. Bellinger wrote:
> > > On Thu, 2011-11-24 at 02:05 +0100, Jörn Engel wrote:
> > > > A reader should spend an extra moment whenever noticing a cast.
> >
> > > > @@ -821,6 +820,7 @@ int iscsi_target_setup_login_socket(
> > > > /*
> > > > * Set SO_REUSEADDR, and disable Nagel Algorithm with TCP_NODELAY.
> > > > */
> > > > + /* FIXME: Someone please explain why this is endian-safe */
> > > > opt = 1;
> > > > if (np->np_network_transport == ISCSI_TCP) {
> > > > ret = kernel_setsockopt(sock, IPPROTO_TCP, TCP_NODELAY,
> > > > @@ -832,6 +832,7 @@ int iscsi_target_setup_login_socket(
> > > > }
> > > > }
> > > >
> > > > + /* FIXME: Someone please explain why this is endian-safe */
> > > > ret = kernel_setsockopt(sock, SOL_SOCKET, SO_REUSEADDR,
> > > > (char *)&opt, sizeof(opt));
> > > > if (ret < 0) {
> > >
> > > Another good catch. Changing opt to be a single char value with
> > > kernel_setsockopt().
> >
> > Sorry, my last change here to use a single char actually actually broke
> > iscsi-target in lio-core, so reverting this for the moment.
> >
> > All of the other net/ users of kernel_setsockopt() seem to be doing the
> > same thing with int when enabling a '1' value opt. Is the caller really
> > not endian safe here..? I don't recall having an issue with big-endian
> > here in the recent past, but i'll look at this with pseries soon to
> > double check since it's been mentioned.
>
> I've written a quick test:
>
> #include <stdio.h>
> int main(void)
> {
> int i, *pi;
> char *pc;
>
> for (i = 0; i < 2; i++) {
> pi = &i;
> pc = (char *)pi;
> *pc;
> printf("%d %d %d %p %p\n", i, *pi, *pc, pi, pc);
> }
> }
>
> Results on x86:
> 0 0 0 0x7fffccb1b3ec 0x7fffccb1b3ec
> 1 1 1 0x7fffccb1b3ec 0x7fffccb1b3ec
>
> Results on PowerPC:
> 0 0 0 2ff22ba0 2ff22ba0
> 1 1 0 2ff22ba0 2ff22ba0
>
> Unless having a different result depending on endianness is the
> desired behaviour, I would call this a problem. ;)
>
If kernel_setsockopt() has a problem with accepting int to enable
boolean *opt usage, then the other mainline usage needs to be fixed as
well. (DaveM CC'ed)
Anyways, will get this verified soon with iscsi-target on ppc64.
Thanks,
--nab
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
