← Back to team overview

kicad-developers team mailing list archive

Re: PATCH: 3D resolver

 

This is why ExpandEnvVarSubstitutions() in common.cpp is protected by a
MUTEX for just this reason.  This was done specifically for loading the
footprint libraries using multiple threads so it is thread safe.

On 4/28/2016 5:05 PM, Cirilo Bernardo wrote:
> Hi Wayne,
> 
>  In fp_lib_table.cpp the FP_LIB_TABLE::ExpandEnvVarSubstitutions
> invokes ExpandEnvVarSubstitutions in common.cpp. In common.cpp
> there is this note:
> 
> // wxGetenv( wchar_t* ) is not re-entrant on linux.
> 
> That sort of thing makes me paranoid. Unfortunately POSIX getenv()
> is not required to be reentrant and yet GNU don't seem to provide
> a reentrant option or better still just make it reentrant. It will take me
> a little more time to convert the wxFileName::Normalize() instances
> to use ExpandEnvVarSubstitutions. Ideally wxWidgets should put a
> mutex on calls to getenv(), but those changes will take a long time
> to get to users.
> 
> - Cirilo
>  
> 
> On Thu, Apr 28, 2016 at 8:44 AM, Wayne Stambaugh <stambaughw@xxxxxxxxx
> <mailto:stambaughw@xxxxxxxxx>> wrote:
> 
>     Cirilo,
> 
>     We already have code that expands environment variables.  It's the
>     static function:
> 
>     const wxString FP_LIB_TABLE::ExpandSubstitutions( const wxString&
>     aString )
> 
>     I would prefer that you use it rather than write your own unless it
>     doesn't suite your needs.  Looking at your patch, it should work
>     just fine.
> 
>     Thanks,
> 
>     Wayne
> 
> 
>     On 4/24/2016 2:06 AM, Cirilo Bernardo wrote:
>     > The attached patch fixes a cut/paste syntactic error present
>     > in all previous patches. A do{} block was changed to if{} but
>     > the while() expression at the end was not removed.
>     >
>     > - Cirilo
>     >
>     >
>     > On Sun, Apr 24, 2016 at 9:06 AM, Cirilo Bernardo
>     > <cirilo.bernardo@xxxxxxxxx <mailto:cirilo.bernardo@xxxxxxxxx>
>     <mailto:cirilo.bernardo@xxxxxxxxx
>     <mailto:cirilo.bernardo@xxxxxxxxx>>> wrote:
>     >
>     >     The attached revised patch (against r6710) adds support for $(ENV_VAR)
>     >     and fixes the bug reported by easyw: "invalid filename" on MSWin when
>     >     manually editing a file name containing ${ENV_VAR}.
>     >
>     >     - Cirilo
>     >
>     >
>     >     On Sat, Apr 23, 2016 at 5:29 PM, jp charras <jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>
>     >     <mailto:jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>>> wrote:
>     >
>     >         Le 23/04/2016 09:16, Cirilo Bernardo a écrit :
>     >         > Thanks Jean-Pierre,  I'll go through the 3D model code and make sure it can
>     >         > use "${" and "$(".  I'm not sure about "%{", "%(" though since that wouldn't
>     >         > be portable.  In principle there can also be multiple substitutions and a
>     >         > substitution in the middle of a string such as
>     >         > "/path/${VAR1}/${VAR2}/model.wrl"
>     >         > but I think the current system should work fine; if people do something else
>     >         > they can be responsible for the resulting behavior.
>     >         >
>     >         > - Cirilo
>     >
>     >         Supporting both "${" and "$(" is enough for me.
>     >         If "${" is the starting point, "}" is the end of var name, and
>     >         for "$(" the end of var name is ")"
>     >
>     >
>     >         >
>     >         > On Sat, Apr 23, 2016 at 4:28 PM, jp charras
>     >         <jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>
>     <mailto:jp.charras@xxxxxxxxxx <mailto:jp.charras@xxxxxxxxxx>>> wrote:
>     >         >
>     >         >> Le 23/04/2016 01:29, Cirilo Bernardo a écrit :
>     >         >>> Hi Wayne,
>     >         >>>
>     >         >>>  The attached patch was made against r6709.
>     >         >>>
>     >         >>> - Cirilo
>     >         >>
>     >         >> Cirilo, I just had a look at the patch, and noticed a
>     (minor)
>     >         issue:
>     >         >> you are searching an ENV VAR by searching "${"
>     >         >>
>     >         >> This is perfectly true, but incomplete.
>     >         >> wxWidgets accepts both "${" and "$(". (and on windows
>     accept
>     >         also "%{" and
>     >         >> "%(" and more)
>     >         >> The delimiters can be { and } or ( and )
>     >         >> They are equivalent in wxWidgets, and allows using ) or
>     } in
>     >         paths
>     >         >>
>     >         >> (have a look at wxString wxExpandEnvVars(const
>     wxString& str) in
>     >         >> src/common/config.cpp, perhaps it
>     >         >> could be used )
>     >         >>
>     >         >>
>     >         >> This in important because we are using both in Kicad.
>     >         >> for instance the fp lib wizard uses $(xxx) and in docs
>     we use
>     >         ${xxx}
>     >         >> notation
>     >         >>
>     >         >> Thanks.
>     >         >>
>     >         >>>
>     >         >>>
>     >         >>> On Sat, Apr 23, 2016 at 12:52 AM, Wayne Stambaugh
>     >         <stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>
>     <mailto:stambaughw@xxxxxxxxx <mailto:stambaughw@xxxxxxxxx>>>
>     >         >>> wrote:
>     >         >>>
>     >         >>>> Cirilo,
>     >         >>>>
>     >         >>>> This patch no longer applies cleanly.  Would you please fix
>     >         it an repost
>     >         >>>> it.
>     >         >>>>
>     >         >>>> Thanks,
>     >         >>>>
>     >         >>>> Wayne
>     >         >>>>
>     >         >>
>     >         >>
>     >         >> --
>     >         >> Jean-Pierre CHARRAS
>     >         >>
>     >         >> _______________________________________________
>     >         >> Mailing list: https://launchpad.net/~kicad-developers
>     >         >> Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     >         <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>>
>     >         >> Unsubscribe : https://launchpad.net/~kicad-developers
>     >         >> More help   : https://help.launchpad.net/ListHelp
>     >         >>
>     >         >
>     >
>     >
>     >         --
>     >         Jean-Pierre CHARRAS
>     >
>     >
>     >
>     >
>     >
>     > _______________________________________________
>     > Mailing list: https://launchpad.net/~kicad-developers
>     > Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     > Unsubscribe : https://launchpad.net/~kicad-developers
>     > More help   : https://help.launchpad.net/ListHelp
>     >
> 
> 
>     _______________________________________________
>     Mailing list: https://launchpad.net/~kicad-developers
>     Post to     : kicad-developers@xxxxxxxxxxxxxxxxxxx
>     <mailto:kicad-developers@xxxxxxxxxxxxxxxxxxx>
>     Unsubscribe : https://launchpad.net/~kicad-developers
>     More help   : https://help.launchpad.net/ListHelp
> 
> 



References