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

fixed compile error in gcc version 4.1.2 #1

Closed
wants to merge 1 commit into from

Conversation

astroshim
Copy link

Hello.
I faced the problems while compiling this module with gcc version 4.1.2 and nginx-1.6.0.
And then i fixed codes.

....
-o objs/addon/ngx_kafka_module/ngx_http_kafka_module.o
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c
cc1: warnings being treated as errors
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c: In function ‘ngx_http_kafka_main_conf_broker_add’:
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c:162: warning: value computed is not used
make[1]: *** [objs/addon/ngx_kafka_module/ngx_http_kafka_module.o] Error 1
make[1]: Leaving directory `/tmp/nginx/nginx-1.6.0'
make: *** [build] Error 2

....
make -f objs/Makefile
make[1]: Entering directory /tmp/nginx/nginx-1.6.0' cc -c -pipe -O -W -Wall -Wpointer-arith -Wno-unused-parameter -Werror -g -I/usr/local/include/hiredis/ -I src/core -I src/event -I src/event/modules -I src/os/unix -I objs -I src/http -I src/http/modules -I src/http/modules/perl -I src/mail \ -o objs/addon/ngx_kafka_module/ngx_http_kafka_module.o \ /tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c cc1: warnings being treated as errors /tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c: In function ‘ngx_http_kafka_post_callback_handler’: /tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c:285: warning: ‘main_conf’ may be used uninitialized in this function make[1]: *** [objs/addon/ngx_kafka_module/ngx_http_kafka_module.o] Error 1 make[1]: Leaving directory/tmp/nginx/nginx-1.6.0'
make: *** [build] Error 2

@brg-liuwei
Copy link
Owner

Hi,
I’m very happy you use my nginx kafka module. I found you modified 2 lines, and I wana discuss some points about it.

at line 162: I found that you change ngx_copy to memcpy. I don’t suggest you use memcpy here, because in nginx code style, nginx author use ngx_copy / ngx_memcpy to replace memcpy. You can see that in nginx source code: src/ngx_string.h. I cannot find nginx-1.6.0 in nginx-web-site: http://nginx.org/en/download.html, so, I don’t know if there are some changes about ngx_copy in version 1.6.0. But as we cannot find this version in nginx-web-site, I think maybe it is an un-stable version. Maybe you can use nginx-1.4.7, nginx-1.6.2, nginx-1.7.6 or nginx openrestry bundle(http://openresty.org/).

And I compiled nginx1.6.2 with error warning: "value computed is not used". See core/ngx_string.h, ngx_copy is defined to ngx_cpymem. the difference between ngx_cpymem and ngx_memcpy is that ngx_cpymem add n for its return value. And for our code, we don’t care about the return value of ngx_cpymem. So, replace ngx_copy by ngx_memcpy, the problem can be solved.

at line 286: you add main_conf = NULL when main_conf was declared. I read nginx source code and I found that nginx author DON’T like declare var with assignment. So, I changes this assignment in other place, and at end of this function, before the call of rd_kafka_poll, we must judge if main_conf != NULL, as there is a goto statement at line 308. I think it is better we keep our ngx-module code style with nginx source code.

I have committed the changes to my github.
I compile this module successfully with nginx-1.2.6/nginx-1.4.7 on my MacOS 10.9, and nginx-1.2.6/nginx-1.4.7/nginx-1.6.2 on ubuntu (gcc 4.6.3)

thank you for your bug fix commit.
btw, I am pool in english, wish you can understand my words. lol.

在 2014年10月24日,9:18,astroshim [email protected] 写道:

Hello.
I faced the problems while compiling this module with gcc version 4.1.2 and nginx-1.6.0.
And then i fixed codes.

....
-o objs/addon/ngx_kafka_module/ngx_http_kafka_module.o
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c
cc1: warnings being treated as errors
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c: In function ‘ngx_http_kafka_main_conf_broker_add’:
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c:162: warning: value computed is not used
make[1]: *** [objs/addon/ngx_kafka_module/ngx_http_kafka_module.o] Error 1
make[1]: Leaving directory `/tmp/nginx/nginx-1.6.0'
make: *** [build] Error 2

....
make -f objs/Makefile
make[1]: Entering directory /tmp/nginx/nginx-1.6.0'
cc -c -pipe -O -W -Wall -Wpointer-arith -Wno-unused-parameter -Werror -g -I/usr/local/include/hiredis/ -I src/core -I src/event -I src/event/modules -I src/os/unix -I objs -I src/http -I src/http/modules -I src/http/modules/perl -I src/mail
-o objs/addon/ngx_kafka_module/ngx_http_kafka_module.o
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c
cc1: warnings being treated as errors
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c: In function ‘ngx_http_kafka_post_callback_handler’:
/tmp/nginx/ngx_kafka_module/ngx_http_kafka_module.c:285: warning: ‘main_conf’ may be used uninitialized in this function
make[1]: *** [objs/addon/ngx_kafka_module/ngx_http_kafka_module.o] Error 1
make[1]: Leaving directory/tmp/nginx/nginx-1.6.0'
make: *** [build] Error 2

You can merge this Pull Request by running

git pull https://github.com/astroshim/ngx_kafka_module master
Or view, comment on, or merge it at:

#1

Commit Summary

fixed compile error in gcc version 4.1.2
File Changes

M ngx_http_kafka_module.c (5)
Patch Links:

https://github.com/brg-liuwei/ngx_kafka_module/pull/1.patch
https://github.com/brg-liuwei/ngx_kafka_module/pull/1.diff

Reply to this email directly or view it on GitHub.

@brg-liuwei brg-liuwei closed this Oct 24, 2014
@astroshim
Copy link
Author

Hello.
I am very happy to use your module and really appreciate you. ^^

at line 162: you are right. memcpy function should be changed to ngx_memcpy.
at line 308: if main_conf != NULL code is needed. I agreed with you.

I also compile this module successfully with nginx-1.6.0 on CentOS 5.8 (gcc version 4.1.2).
Thanks.

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.

2 participants