Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

fix a handful of C compiler warnings #45

Merged
merged 2 commits into from

2 participants

@eklitzke

Here's the commit message for the change:

There are a few fixes in here:
 * read(3) returns a regular ssize_t; don't use Py_ssize_t
 * Request_parse wants a size_t, not an ssize_t
 * server_run takes no arguments, so declare it void
 * tp_newfunc should have type (newfunc) (i.e. takes PyTypeObject* as its first
   parameter, instead of PyObject*)

After this change, there are still the following warnings:

  • the constness issue with the on_path callback; I already filed an issue for this
  • there are some Python types that are declared without all parameters in the struct (notably in bjoern/filewrapper.c and bjoern/wsgi.c. I think they'd all be NULL anyway, so I think it's fine to ignore this. Fixing this is easy though, it just requires a bunch of copy-and-paste code to declare the fields.
  • clang (but not GCC) warns about certain unused parameters in methods; it looks like fixing these requires declaring the parameters with __attribute__((unused)) which I think is unportable (and kind of ugly!)
@eklitzke eklitzke fix a handful of C compiler warnings
There are a few fixes in here:
 * read(3) returns a regular ssize_t; don't use Py_ssize_t
 * Request_parse wants a size_t, not an ssize_t
 * server_run takes no arguments, so declare it void
 * tp_newfunc should have type (newfunc) (i.e. takes PyTypeObject* as its first
   parameter, instead of PyObject*)
241b3ed
@jonashaag
Owner

Thanks for all the effort, dude! BUT.... ;-)

  • isn't Py_ssize_t an alias to ssize_t (on platforms that have ssize_t at least)?
  • I agree that (newfunc) should be used; however, shouldn't the first argument of FileWrapper_New be a PyTypeObject* too, then? Why doesn't that throw a warning?

I don't know how to silence warnings about thestruct/NULL "padding" thing -- I won't add dozens of NULL, NULL, ... lines.

Warnings about unused parameters are silenced in GCC, not sure if clang recognizes -Wno-unused, though.

@eklitzke

OK, I pushed the following changes to this branch:

  • I updated my branch to use PyTypeObject* instead of casting using newfunc
  • I added -Wno-missing-field-initializers to the setup.py to suppress the field initializers warnings, since it doesn't make sense to show them if they're not going to be fixed.

Also, Py_ssize_t is an alias to ssize_t, but I personally think it's weird to use a Python typedef for a variable that doesn't interact with any of the Python API stuff (in this case the variable comes out of read(3), and isn't used by Python API functions). Py_ssize_t also doesn't work on Python 2.4, but that's probably not an issue since Python 2.4 is so ancient (it predates the WSGI stuff anyway).

BTW thanks for dealing with all of these tiny pull requests that don't really add functionality -- I want to experiment with some other features using bjoern as a platform, so these small pull requests are just things I picked up while learning the code base.

@jonashaag jonashaag merged commit daac66e into jonashaag:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Sep 18, 2011
  1. @eklitzke

    fix a handful of C compiler warnings

    eklitzke authored
    There are a few fixes in here:
     * read(3) returns a regular ssize_t; don't use Py_ssize_t
     * Request_parse wants a size_t, not an ssize_t
     * server_run takes no arguments, so declare it void
     * tp_newfunc should have type (newfunc) (i.e. takes PyTypeObject* as its first
       parameter, instead of PyObject*)
Commits on Sep 19, 2011
  1. @eklitzke
This page is out of date. Refresh to see the latest.
View
2  bjoern/filewrapper.c
@@ -1,7 +1,7 @@
#include "filewrapper.h"
static PyObject*
-FileWrapper_New(PyObject* self, PyObject* args, PyObject* kwargs)
+FileWrapper_New(PyTypeObject* self, PyObject* args, PyObject* kwargs)
{
PyObject* file;
if(!PyArg_ParseTuple(args, "O:FileWrapper", &file))
View
6 bjoern/server.c
@@ -37,7 +37,7 @@ static bool send_chunk(Request*);
static bool do_sendfile(Request*);
static bool handle_nonzero_errno(Request*);
-void server_run(const char* hostaddr, const int port)
+void server_run(void)
{
struct ev_loop* mainloop = ev_default_loop(0);
@@ -131,7 +131,7 @@ ev_io_on_read(struct ev_loop* mainloop, ev_io* watcher, const int events)
Request* request = REQUEST_FROM_WATCHER(watcher);
- Py_ssize_t read_bytes = read(
+ ssize_t read_bytes = read(
request->client_fd,
read_buf,
READ_BUFFER_SIZE
@@ -152,7 +152,7 @@ ev_io_on_read(struct ev_loop* mainloop, ev_io* watcher, const int events)
goto out;
}
- Request_parse(request, read_buf, read_bytes);
+ Request_parse(request, read_buf, (size_t)read_bytes);
if(request->state.error_code) {
DBG_REQ(request, "Parse error");
View
2  bjoern/server.h
@@ -1,4 +1,4 @@
#include "request.h"
bool server_init(const char* hostaddr, const int port);
-void server_run();
+void server_run(void);
View
3  setup.py
@@ -13,7 +13,8 @@
define_macros = [('WANT_SENDFILE', '1'),
('WANT_SIGINT_HANDLING', '1')],
extra_compile_args = ['-std=c99', '-fno-strict-aliasing', '-Wall',
- '-Wextra', '-Wno-unused', '-g', '-fPIC']
+ '-Wextra', '-Wno-unused', '-g', '-fPIC',
+ '-Wno-missing-field-initializers']
)
setup(
Something went wrong with that request. Please try again.