Add data pointer to listener struct.

[brucespang]

This allow us to preserve more state about the listener object when we handle an accept.

For example, I have a module that handles http requests a bit like sinatra. I'd like to have multiple instances of this module, so that a request to / on two different ports can be handled two different ways. Without being able to store data in the listener object, there's no nice way to figure out how we should handle a given request.

[wez]

Can you expand on how this is used? When it comes to application specific helpers like this, I generally think that it is best that we have some explicit function calls to manage the data; that makes it easier to discover and learn how to use it because it is easier to add a function to the docs.

Is the data effectively a constant struct that you initialize and pass in at ph_listener_new time? Is it expected to be mutated throughout the lifetime of the process?

Wondering if we should label this as acceptor_data and add ph_listener_(get|set)_acceptor_data() accessors for it.

[brucespang]

The structure is initialized at startup, around the same time that ph_listener_new is called. It does not change during the lifetime of the process.

That naming makes sense, and it would be good to have accessors. I can update the pull request, if you'd like.

[wez]

That would be great; also great to include some docs with the declaration of the accessor functions in the header file, and super extra points for including an example of creating a listener and setting the data, then consuming it in the acceptor function

[brucespang]

Will do. Would you prefer the example to be in the header documentation or examples/?

[wez]

in the header docs is best, because that will show up at http://facebook.github.io/libphenom when we build and push the docs.

Take a look at http://facebook.github.io/libphenom/#string--ph_string_iterate_utf8_as_utf16 for an example; the source for that is in string.h

The docs should build locally as part of make if you have PHP available; you can then open docs/index.html in a browser to see how they look

[wez]

Sorry; I missed that you updated this. Looks good except for one tiny nit; we're using type *var rather than type* var throughout the rest of the codebase; please tweak to match, and I'll land this :-)

[brucespang]

Ah, so you are! I think I've got them all changed, please let me know if I've missed something.

Thanks!

[wez]

Sorry for the delay; pulled in via hub am https://github.com/facebook/libphenom/pull/50.
Thanks for your contribution!

[brucespang]

Thanks for merging this!

On Feb 15, 2014, at 5:13 PM, Wez Furlong notifications@github.com wrote:

Closed #50.

—
Reply to this email directly or view it on GitHub.