On Tue, 03 Mar 2009 11:49:56 +0200
Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> FUJITA Tomonori wrote:
> > On Tue, 03 Mar 2009 10:36:40 +0200
> > Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> >
> >> FUJITA Tomonori wrote:
> >>> On Sun, 1 Mar 2009 18:51:53 +0200
> >>> Boaz Harrosh <bharrosh@xxxxxxxxxxx> wrote:
> >>>
> >>>> The BSD's bind does not like the Linux IPC names with zero
> >>>> as first char and zeros padding at end. Separate implementations
> >>>> in os.c.
> >>>>
> >>>> Note: On BSD, after exit a file is left in current directory
> >>>> with the pipe's name. The "up" script was modified to
> >>>> delete this file before and after the tgtd load.
> >>>> (File name is: TGT_IPC_ABSTRACT_NAMESPACE)
> >>>>
> >>>> Signed-off-by: Boaz Harrosh <bharrosh@xxxxxxxxxxx>
> >>>> ---
> >>>> usr/bsd/os.c | 14 ++++++++++++++
> >>>> usr/linux/os.c | 10 ++++++++++
> >>>> usr/mgmt.c | 6 ++----
> >>>> usr/os.h | 3 +++
> >>>> 4 files changed, 29 insertions(+), 4 deletions(-)
> >>> Hmm, the following patch seems to work on both Linux and FreeBSD 7.1
> >>> for me.
> >>>
> >>> Yeah, I installed FreeBSD 7.1 to try to write the better patches.
> >>>
> >>>
> >>> diff --git a/usr/mgmt.c b/usr/mgmt.c
> >>> index 5351b2c..47c8820 100644
> >>> --- a/usr/mgmt.c
> >>> +++ b/usr/mgmt.c
> >>> @@ -562,8 +562,7 @@ int ipc_init(void)
> >>>
> >>> memset(&addr, 0, sizeof(addr));
> >>> addr.sun_family = AF_LOCAL;
> >>> - memcpy((char *) &addr.sun_path + 1, TGT_IPC_NAMESPACE,
> >>> - strlen(TGT_IPC_NAMESPACE));
> >>> + strncpy(addr.sun_path, TGT_IPC_NAMESPACE, sizeof(addr.sun_path));
> >>>
> >>> err = bind(fd, (struct sockaddr *) &addr, sizeof(addr));
> >>> if (err) {
> >>> diff --git a/usr/tgtadm.c b/usr/tgtadm.c
> >>> index 6280c99..71d7fc5 100644
> >>> --- a/usr/tgtadm.c
> >>> +++ b/usr/tgtadm.c
> >>> @@ -182,8 +182,7 @@ static int ipc_mgmt_connect(int *fd)
> >>>
> >>> memset(&addr, 0, sizeof(addr));
> >>> addr.sun_family = AF_LOCAL;
> >>> - memcpy((char *) &addr.sun_path + 1, TGT_IPC_NAMESPACE,
> >>> - strlen(TGT_IPC_NAMESPACE));
> >>> + strncpy(addr.sun_path, TGT_IPC_NAMESPACE, sizeof(addr.sun_path));
> >>>
> >>> err = connect(*fd, (struct sockaddr *) &addr, sizeof(addr));
> >>> if (err < 0)
> >>
> >> Sure, thanks
> >> I did not want to change the Linux code, I have no experience
> >> with IPC code, I thought it is important to have zero at first char,
> >> otherwise why is it there?
> >
> > I copied open-iscsi code without thinking, as you did. But I
> > investigated the proper way this time instead of applying a hacky
> > workaround patch.
> >
> >
> >> Feel free to replace the patch with this one
> >
> > Well, seems that I need to replace more.
>
> Well I have only tested what I need - OSD. We don't have to fix everything
> at once. As long as the Linux code is unchanged, the BSD code can go in Little
Huh? This is a bug. We should fix the Linux code.
> by little. Please accept this stage that can at least compile on BSD.
> And we can incrementally fix the BSD bugs after that.
I will not merge such hacky patches. I care about the code quality. As
I said before, the reason why I'm adding BSD support is improving tgt.
--
To unsubscribe from this list: send the line "unsubscribe stgt" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
