zserge / jsmn

Jsmn is a world fastest JSON parser/tokenizer. This is the official repo replacing the old one at Bitbucket
MIT License
3.65k stars 778 forks source link

Problem with multiple definitions on C++ #171

Open the-mush opened 5 years ago

the-mush commented 5 years ago

Hi, I'm using JSMN in an Arduino project using C++, including it in a header file which gets called in multiple files on the project. The only way I could get it to work is if I modify the jsmn_init and jsmn_parse by adding the inline tag (doing so seems to work ok for now). My questions are:

pt300 commented 5 years ago

Would you mind sharing exact errors you are getting? I think I see what might possibly cause the problem you are describing but I need something more to give me confidence.

the-mush commented 5 years ago

Yes! To be more precise, I'm using Platform.IO and including JSMN from their Library Manager. The error I'm getting is: .pio\build\firebeetle32\src\main.cpp.o: In function 'jsmn_parse': main.cpp:(.text.jsmn_parse+0x0): multiple definition of 'jsmn_parse' .pio\build\firebeetle32\src\fatherFile.cpp.o:fatherFile.cpp:(.text.jsmn_parse+0x0): first defined here .pio\build\firebeetle32\src\main.cpp.o: In function 'jsmn_init': main.cpp:(.text.jsmn_init+0x0): multiple definition of 'jsmn_init' .pio\build\firebeetle32\src\fatherFile.cpp.o:fatherFile.cpp:(.text.jsmn_init+0x0): first defined here .pio\build\firebeetle32\src\sonFile.cpp.o: In function 'jsmn_parse': sonFile.cpp:(.text.jsmn_parse+0x0): multiple definition of 'jsmn_parse' .pio\build\firebeetle32\src\fatherFile.cpp.o:fatherFile.cpp:(.text.jsmn_parse+0x0): first defined here .pio\build\firebeetle32\src\sonFile.cpp.o: In function 'jsmn_init': sonFile.cpp:(.text.jsmn_init+0x0): multiple definition of 'jsmn_init' .pio\build\firebeetle32\src\fatherFile.cpp.o:fatherFile.cpp:(.text.jsmn_init+0x0): first defined here collect2.exe: error: ld returned 1 exit status *** [.pio\build\firebeetle32\firmware.elf] Error 1

An example structure to reproduce my problem is composed of 5 files: main.cpp - Where I have the main Arduino functions setup() and loop(). Includes only son.h father.h - where I originally use JSMN functions and datatypes. Includes only jsmn.h father.cpp - includes only father.h son.h - includes only father.h son.cpp - includes only son.h

pt300 commented 5 years ago

I'll send a patch file in a second and please tell me if it helps

pt300 commented 5 years ago

Here's the patch fix_guard.txt

the-mush commented 5 years ago

Oh, ok... sorry for the noob question, but how do I use it?

pt300 commented 5 years ago

Considering how small the change is you can do it by hand. On line 103 after #ifndef JSMN_HEADER add a line #define JSMN_HEADER. You should end up with something like this obraz

the-mush commented 5 years ago

For what I understand from the patch file, you are trying to make sure that the user has defined JSMN_HEADER, and now re-reading the README I've notice the Usage section. If I follow those instructions it does seem to compile just fine now! Probably that was all I was missing?

the-mush commented 5 years ago

What is a pity though, is that I still need to create a jsmn file in every project I need the library, just that instead of the .h file with the inline tags, now it's just an almost empty .c file.

Wouldn't this patch go against the suggestion on the Usage section? as it seems to nullify the definition outside of the library

pt300 commented 5 years ago

Have you tested that change without your own? I feel like it might not be enough though and that it's a problem with how the library works now.

the-mush commented 5 years ago

Ok, now just tried the change in your last comment and it does not link properly (gives exact same error as before).

pt300 commented 5 years ago

oh nooo, I am sorry. I'm feeling quite stupid. Guess it's a terrible idea to deal with stuff like this when one's tired. Disregard what I've said so far. Just make sure that in all the files where you #include jsmn except one you do #define JSMN_HEADER before the include. This should ensure that exists only one definition of the functions and rest of your code jsut gets the declarations.

pt300 commented 5 years ago

Make sure to revert all the code changes. I am again really sorry.

the-mush commented 5 years ago

another detail I forgot to mention is that I'm protecting father.h and son.h just with #pragma once, if that makes any difference

the-mush commented 5 years ago

Oh wow! Didn't notice that I can just use the define without creating the .c file. It was just that simple. Tank you for your help and get some well deserved rest 😉!

pt300 commented 5 years ago

I'd say you could do

#define JSMN_HEADER
#include "jsmn.h"

in father.h and once include jsmn.h before including father.h in main.cpp

the-mush commented 5 years ago

Maybe I would just refrase the comment:

`/ Additionally, create one jsmn.c file for jsmn implementation: /

include "jsmn.h"`

To Optionally, create one jmsn.c file ...

the-mush commented 5 years ago

I'd say you could do

#define JSMN_HEADER
#include "jsmn.h"

in father.h and once include jsmn.h before including father.h in main.cpp

It seems to work ok without including jsmn.h before father.h 🤷‍♂️🤔

WillNilges commented 4 years ago

A year later, this is... kinda still an issue :/

I'm working on a little util in C with multiple C files and I'm getting the same kinds of issues.

[~/Documents/Code/octo-dash-curses] [±headers✗] 
$ make -B
  CC      main.c
cc   -o build/main.o -c src/main.c
  CC      util.c
cc   -o build/util.o -c src/util.c
gcc -o octo -lcurl build/main.o build/util.o -I.
/usr/bin/ld: build/util.o: in function `jsmn_parse':
util.c:(.text+0x5a6): multiple definition of `jsmn_parse'; build/main.o:main.c:(.text+0x4d4): first defined here
/usr/bin/ld: build/util.o: in function `jsmn_init':
util.c:(.text+0xa8a): multiple definition of `jsmn_init'; build/main.o:main.c:(.text+0x9b8): first defined here
collect2: error: ld returned 1 exit status
make: *** [Makefile:4: octo] Error 1

here is my code if you are so inclined to view it.

WillNilges commented 4 years ago

Fixed it by using code from particle.

PKD667 commented 2 years ago

@WillNilges the link is broken , i tried to search your repo for the jsmn file but it seems like you replaced it by cjson. If you still have it can you please share your solution ?

WillNilges commented 2 years ago

Unfortunately, I can't say I remember much about the details of that project. I think I switched to cjson because it worked better for my needs.

I can tell you when I removed jsmn: https://github.com/WillNilges/octo-dash-curses/commit/545e7f97eecbdd150a9f0f33d76a1e0e489051b6

This could also be useful: https://github.com/WillNilges/octo-dash-curses/commit/8e9def120455d7993d00ad67fed0593613ce5905

You might try looking around there to see if you can find it. Sorry, I shoulda hard-linked to the commit 2 years ago 😂

PKD667 commented 2 years ago

thanks , i'll try to find the solution

Le dim. 14 août 2022 à 16:11, Willard Nilges @.***> a écrit :

Unfortunately, I can't say I remember much about the details of that project. I think I switched to cjson because it worked better for my needs.

I can tell you when I removed jsmn: @.*** https://github.com/WillNilges/octo-dash-curses/commit/545e7f97eecbdd150a9f0f33d76a1e0e489051b6

This could also be useful: @.*** https://github.com/WillNilges/octo-dash-curses/commit/8e9def120455d7993d00ad67fed0593613ce5905

You might try looking around there to see if you can find it. Sorry, I shoulda hard-linked to the commit 2 years ago 😂

— Reply to this email directly, view it on GitHub https://github.com/zserge/jsmn/issues/171#issuecomment-1214386232, or unsubscribe https://github.com/notifications/unsubscribe-auth/ARPSUUSUH5OSUL35DQ33MYTVZD5CJANCNFSM4IVXR3JA . You are receiving this because you commented.Message ID: @.***>

pt300 commented 2 years ago

It seems like the move to single header was not the best idea. It somewhat makes sense in small embedded projects, but seems to just cause issues in bigger projects.

Do you think it would be better to just revert to the regular source + headers system or somehow maintain both versions?

BenBE commented 2 years ago

It seems like the move to single header was not the best idea. It somewhat makes sense in small embedded projects, but seems to just cause issues in bigger projects.

While single-file libraries make sense with some build systems, they more often than not just are a PITA as they require to define custom compilation flags for one file, but not any other. The much simpler approach really is having the main code part of the library as part of the C files, which can then be compiled once and just linked into the final project (be it directly or indirectly using an object archive (static linking) or even as shared library (dynamic linking). This automatically resolves any symbol duplication issues that otherwise improper management of compilation flags with the single header approach may cause. Additionally, the header files are called "headers" for a reason: They should define the externally visible interface of a library, not its internal implementation.

Do you think it would be better to just revert to the regular source + headers system or somehow maintain both versions?

Yes, please have the code be a normal C file, with the header solely containing the externally visible library interface (and nothing more).