FRiCKLE / ngx_postgres

upstream module that allows nginx to communicate directly with PostgreSQL database.
http://labs.frickle.com/nginx_ngx_postgres/
BSD 2-Clause "Simplified" License
545 stars 122 forks source link

Fixing compilation with nginx>=1.9.1 #38

Closed msva closed 9 years ago

agentzh commented 9 years ago

@msva Yes, like that. But you'd better ensure empty lines around your #if, #else, and #endif for aesthetic reasons.

msva commented 9 years ago

here it is ;)

agentzh commented 9 years ago

@msva I think you need to short-circuit the "tv" variable declaration for nginx 1.9.1+ too. It is never used in that code path.

Also, again: you'd better ensure empty lines around your #if, #else, and #endif for aesthetic reasons.

@PiotrSikora ?

msva commented 9 years ago

will it be okay in that way?

agentzh commented 9 years ago

@msva No, putting the variable declaration inside the if block also violates the nginx coding style (well, I didn't invent that style).

msva commented 9 years ago

Uhm.. But according to that: http://wiki.nginx.org/CodingStyle (although, there is struct, but not plain variables) it is okay... But ok, I'll think about it ;)

msva commented 9 years ago

Uhm... Did I misunderstood you about what exactly violating style, or do NginX team violates style theyself? http://hg.nginx.org/nginx/file/6893a1007a7c/src/http/ngx_http_request.c#l204

agentzh commented 9 years ago

@msva Regarding the empty lines around macro directives, either way is fine. That's why I said it was for aesthetic reasons. For example,

http://hg.nginx.org/nginx/file/6893a1007a7c/src/http/ngx_http_request.c#l1833

msva commented 9 years ago

So, is it okay now? :)

agentzh commented 9 years ago

@msva Looking good to me now :)

msva commented 9 years ago

@PiotrSikora ping? :(

agentzh commented 9 years ago

@msva No worries. I'll look into this.

agentzh commented 9 years ago

@msva I'm getting the following compilation error on Linux (Fedora 22, gcc 5.1.1) using your branch:

/home/agentzh/git/postgres-nginx-module/src/ngx_postgres_util.c: In function ‘ngx_postgres_upstream_finalize_request’:
/home/agentzh/git/postgres-nginx-module/src/ngx_postgres_util.c:71:33: error: assignment makes integer from pointer without a cast [-Werror=int-conversion]
         u->state->response_time = ngx_timeofday()->msec - u->state->response_time;
                                 ^
cc1: all warnings being treated as errors

Maybe you should use ngx_current_msec instead of ngx_timeofday()->msec here?

agentzh commented 9 years ago

@msva Never mind, I've committed a slightly modified version of your patch to master. Thanks for your contribution!