Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Made ndpi memory allocator type compatible with C++ on x686 platforms #132

Merged
merged 1 commit into from Dec 31, 2015
Merged

Made ndpi memory allocator type compatible with C++ on x686 platforms #132

merged 1 commit into from Dec 31, 2015

Conversation

pavel-odintsov
Copy link
Contributor

Hello!

nDPI uses unsigned long as allocation functions parameter. It's not recommended way because it's not platform independent. ISO / C++ standard uses size_t for this case.

So I decided to change all "unsigned long" to size_t.

For C++ case it could help critical error about type mismatch on 32 bit platforms (because we could not use 8 byte counter here):

g++ ndpi_example.cpp  -I/opt/ndpi/include/libndpi-1.7.1  -L/opt/ndpi/lib -lndpi  
ndpi_example.cpp: In function ‘ndpi_detection_module_struct* init_ndpi()’:
ndpi_example.cpp:40:89: error: invalid conversion from ‘void* (*)(size_t)throw () {aka void* (*)(unsigned int)throw ()}’ to ‘void* (*)(long unsigned int)’ [-fpermissive]
In file included from ndpi_example.cpp:3:0:
/opt/ndpi/include/libndpi-1.7.1/libndpi/ndpi_api.h:77:40: error:   initializing argument 2 of ‘ndpi_detection_module_struct* ndpi_init_detection_module(u_int32_t, void* (*)(long unsigned int), void (*)(void*), ndpi_debug_function_ptr)’ [-fpermissive]

Thanks!

…. Switch from unsigned long to size_t as allocation functions parameter.
@kYroL01
Copy link
Contributor

kYroL01 commented Dec 28, 2015

@lucaderi I think is correct: size_t is much more portable and avoid any exceptions caused by the different size of types.
For example: http://stackoverflow.com/questions/15637228/what-is-the-downside-of-replacing-size-t-with-unsigned-long

What do you think ?

@pavel-odintsov
Copy link
Contributor Author

I'm also like this idea because for non standard platforms PPS, ARM it could make sense.

@kYroL01
Copy link
Contributor

kYroL01 commented Dec 30, 2015

I'm agree with you but I prefer to wait @lucaderi opinion before merge it ;)

@lucaderi
Copy link
Member

@kYroL01 @pavel-odintsov I think the change is a good idea. As far as I am concerned you can merge it Michele.

kYroL01 added a commit that referenced this pull request Dec 31, 2015
Made ndpi memory allocator type compatible with C++ on x686 platforms
@kYroL01 kYroL01 merged commit a2c51a5 into ntop:dev Dec 31, 2015
@pavel-odintsov
Copy link
Contributor Author

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants