[pgrouting-dev] duplicate function names in core

[Mario Basa]

Hello Steve,

With the problem I am encountering when I brought the driving_distance
into core, both boost_drivedist.cpp and astar_boost_wrapper.cpp have
graph_add_edge() function and Edge and Vertex structs.

The offending components here are the structs I think since they have
different values even if they have the same name in the c++ files. So,
whichever you compile first (astar or drivedist) in the CMAKE file
will work while the other will core dump. For fun, I made a different
library in the CMAKE for drivedist, and it still dumped core.

It was only after I renamed everything (Edge_dd,Vertex_dd, etc.) did
it work. Again, this is so difficult to debug, well time consuming
anyway. I am not so familiar with the recent gcc compiler, but I'll
try look if there is a compile time option to give out warnings for
similar function names.

I'll move the common functions into a pgr_utils.h file to take out
redundancy in core.

Mario.


On Wed, Jul 11, 2012 at 8:38 AM, Stephen Woodbridge
<woodbri at swoodbridge.com> wrote:
> On 7/10/2012 5:23 AM, Mario Basa wrote:
>>
>> Hello,
>>
>> I am encountering problems placing the drivedist.c and
>> boost_drivedist.cpp into core (everything compiles, drivedist works
>> but astar searches dumps core. If I take out drivedist, astar works
>> like normal.), so I decided to poke around the source. I just found
>> out that there are so many duplicate functions (i.e.
>> finish(),text2char(),etc.) and struct names (edge_column,etc.) in each
>> of the C files.
>>
>> Since C does not have any namespace functionality nor function
>> overloading, I don't have any idea which one will be called whenever
>> there is a library call. Scary since this will be difficult to debug.
>>
>> Now that pgRouting is getting source contributions, I think this will
>> be a good time that we start discussing function naming conventions to
>> avoid duplicates as well as directory (core and extensions)
>> structures.
>
>
> Mario,
>
> UGH! Ideally one of two things should happen:
>
> 1. common functions with compatible signatures should be extracted into a
> common.c common.h or utils.c utils.h and get shared by all algorithms that
> need them.
>
> 2. local functions should all be declares "static" which means they are not
> visible or callable from references outside the file they are declared in.
>
> So, if you have a C wrapper and it needs functions and no other files need
> to use these functions then they should be declared as static and placed in
> the C wrapper file.
>
> If you have some function that multiple C files need to call or you C++ also
> needs to call, then it needs to go into a file like <namespace>_utils.c and
> all the functions in this file need to have a unique
> <namespace>_functionname() like signature.
>
> I think this will minimize the changes we need to make to the existing code
> using "static" declarations for most things, and then moving the others into
> some namespace'd set of files.
>
> How does this sound?
>
> I like the idea of having multiple library .so files rather than one huge
> file because it keeps things separated but more so because it makes it easy
> to rebuild one library without effecting any of the others. I would like to
> know that if I change TRSP that there is little to zero change that I can
> impact the other algorithms.
>
> Thank about a production environment where you have pgrouting running on
> lots of databases and in one database you want to add TRSP. By making it one
> big library you force a change on all the databases that did not need that
> change, but if TRSP is in a separate library then all the database that are
> not using it can be assured to be stable because they did not change. The
> worsted cases is that the new database with TRSP added to it might be
> unstable and if there is a collision of files like you describe the cases
> crashes, it only happens in the new database. Hmmm, maybe it doesn't work
> this way once the postgresql loads the library. But I don't remember having
> problems loading multiple libraries.
>
> Thoughts?
>
> -Steve
> _______________________________________________
> pgrouting-dev mailing list
> pgrouting-dev at lists.osgeo.org
> http://lists.osgeo.org/mailman/listinfo/pgrouting-dev