<div dir="ltr"><div>Thanks Alex for your quick response.</div><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Mar 14, 2017 at 11:49 PM, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><span class="gmail-">On 03/14/2017 08:44 AM, Gary Wang wrote:<br>
>     About the DEFAULT_STATEDIR,<br>
>     ...<br>
>     DEFS += -DDEFAULT_STATEDIR=\"$(<wbr>localstatedir)/run/squid\"<br>
>     ...<br>
>     According to the above share memory namespace in snap world, it<br>
> couldn't help on this either.<br>
<br>
</span>Here is how I [mis]interpret the above:<br>
<br>
1. Squid has a DEFAULT_STATEDIR compile-time constant that provides the<br>
right control knob you need but uses the wrong hard-coded value.<br>
<br>
2. You are adding a compile-time variable DEFAULT_IPC_PREFIX that does<br>
what DEFAULT_STATEDIR does but can be changed at ./configure time.<br>
<br>
This raises an obvious question: Why not make DEFAULT_STATEDIR<br>
configurable instead?  (If we do make it configurable, we may need to<br>
renamed it to STATEDIR, IPCDIR, or something like that but that is a<br>
separate and minor issue.)<br>
<br></blockquote><div>   A: Actually, DEFAULT_IPC_PREFIX only changes the shared memory file name</div><div>       not the file path. So the usage of these two options are different. </div><div>       Meanwhile, DEFAULT_STATEDIR only helps the case that the segment name is path.</div><div>       It can't help this situation where segment name is not path e.g.  shared memory is created</div><div>       in /dev/shm folder on Ubuntu.</div><div>       <a href="http://bazaar.launchpad.net/~gary-wzl77/squid/ipc_prefix/view/head:/src/ipc/mem/Segment.cc#L229">http://bazaar.launchpad.net/~gary-wzl77/squid/ipc_prefix/view/head:/src/ipc/mem/Segment.cc#L229</a></div><div>   </div><div>       Another concern is that even though we make DEFAULT_STATEDIR</div>configurable instead of introducing new option, STATEDIR, IPCDIR is not suitable as</div><div class="gmail_quote">end-user might specify the dir path for these options however the purpose of DEFAULT_IPC_PREFIX</div><div class="gmail_quote">is only add prefix to shared memory file name<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
AFAICT, DEFAULT_STATEDIR is also used for IPC communications in<br>
src/ipc/Port.cc. Your patch does not change that code. This means that<br>
<br>
* either you forgot to change src/ipc/Port.cc to honor the new<br>
DEFAULT_IPC_PREFIX<br></blockquote><div>   A: I will change src/ipc/Port.cc in the next commit once we reach an agreement</div><div>       on the usage of compiler option.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
* or src/ipc/Port.cc cannot use DEFAULT_IPC_PREFIX for some reason (and,<br>
hence, DEFAULT_IPC_PREFIX is the wrong name because the parameter does<br>
not apply to some of the IPC code).<br>
<br>
Alex.<br>
<br>
<br>
<br>
> === modified file '<a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>'<br>
> --- <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>      2017-01-28 03:54:15 +0000<br>
> +++ <a href="http://configure.ac" rel="noreferrer" target="_blank">configure.ac</a>      2017-03-02 03:51:46 +0000<br>
> @@ -311,6 +311,15 @@<br>
>  ])<br>
>  AC_SUBST(DEFAULT_SWAP_DIR)<br>
><br>
> +DEFAULT_IPC_PREFIX=""<br>
> +AC_ARG_WITH(ipcprefix,<br>
> +  AS_HELP_STRING([--with-<wbr>ipcprefix=NAME],<br>
> +    [Default namespace prefix for shared memory object for ipc communication.<br>
> +     Default is empty string and the share memory object name will be specified by squid. ]), [<br>
> +   DEFAULT_IPC_PREFIX="$withval"<br>
> +])<br>
> +AC_SUBST(DEFAULT_IPC_PREFIX)<br>
> +<br>
>  if test "$squid_cv_compiler" = "gcc"; then<br>
>    GCCVER=`$CC -v 2>&1 | awk '$2 ==  "version" {print $3}'`<br>
>    GCCVER2=`echo $GCCVER | awk '{print $1 * 100}'`<br>
><br>
> === modified file 'src/ipc/Makefile.am'<br>
> --- src/ipc/Makefile.am       2017-01-01 00:16:45 +0000<br>
> +++ src/ipc/Makefile.am       2017-03-02 03:51:46 +0000<br>
> @@ -68,7 +68,7 @@<br>
>       mem/Segment.cc \<br>
>       mem/Segment.h<br>
><br>
> -DEFS += -DDEFAULT_STATEDIR=\"$(<wbr>localstatedir)/run/squid\"<br>
> +DEFS += -DDEFAULT_STATEDIR=\"$(<wbr>localstatedir)/run/squid\" -DDEFAULT_IPC_PREFIX=\"$(<wbr>DEFAULT_IPC_PREFIX)\"<br>
><br>
>  install-data-local:<br>
>       $(mkinstalldirs) $(DESTDIR)$(localstatedir)/<wbr>run/squid;<br>
><br>
> === modified file 'src/ipc/mem/Segment.cc'<br>
> --- src/ipc/mem/Segment.cc    2017-01-01 00:16:45 +0000<br>
> +++ src/ipc/mem/Segment.cc    2017-03-02 03:51:46 +0000<br>
> @@ -30,6 +30,8 @@<br>
>  #include <unistd.h><br>
>  #endif<br>
><br>
> +#define DEFAULT_SQUID_IPC_PREFIX  "/" DEFAULT_IPC_PREFIX<br>
> +<br>
>  // test cases change this<br>
>  const char *Ipc::Mem::Segment::BasePath = DEFAULT_STATEDIR;<br>
><br>
> @@ -229,9 +231,9 @@<br>
>      if (nameIsPath) {<br>
>          name.append(BasePath);<br>
>          if (name[name.size()-1] != '/')<br>
> -            name.append('/');<br>
> +            name.append(DEFAULT_SQUID_IPC_<wbr>PREFIX);<br>
>      } else {<br>
> -        name.append('/');<br>
> +        name.append(DEFAULT_SQUID_IPC_<wbr>PREFIX);<br>
>          name.append(service_name.c_<wbr>str());<br>
>          name.append('-');<br>
>      }<br>
<br>
</blockquote></div><br><br clear="all"><div><br></div>-- <br><div class="gmail_signature"><div dir="ltr">Br<div>Gary.Wzl</div></div></div>
</div></div>