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. ;)
Jörn
--
My second remark is that our intellectual powers are rather geared to
master static relations and that our powers to visualize processes
evolving in time are relatively poorly developed.
-- Edsger W. Dijkstra
--
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
