dbs [Sun, 25 Oct 2009 21:44:26 +0000 (21:44 +0000)]
Improve the 'help' output for srfsh:
* Document the method-name parameter for the introspect command
* Document the currently supported srfsh variables
* Reorganize the help slightly to place more emphasis on
introspect and request and cut down on some of the blank lines.
scottmk [Sun, 25 Oct 2009 17:02:06 +0000 (17:02 +0000)]
1. Moved several macros from the header to the implementation file. They aren't used
anywhere else.
2. Renamed SERVER_SOCKET and CLIENT_SOCKET to LISTENER_SOCKET and DATA_SOCKET,
respectively. The new names more accurately reflect the uses to which the two
socket types are put. (Note that some so-called CLIENT_SOCKETs were, in fact,
opened by servers.)
3. Changed socket_open_udp_server() to open a DATA_SOCKET (formerly called a
CLIENT_SOCKET) instead of a LISTENER_SOCKET (formerly called a SERVER_SOCKET).
Otherwise an attempt to wait on such a socket would wind up treating it like
a listener. That doesn't work for UDP. In practice this change has no effect,
since no application ever calls this function anyway.
4. Always close a socket before removing the associated socket_node. Otherwise we
will leak sockets in some situations.
5. Tinkered further with the comments, especially in the header file.
M include/opensrf/socket_bundle.h
M src/libopensrf/socket_bundle.c
scottmk [Sun, 25 Oct 2009 05:35:47 +0000 (05:35 +0000)]
Merged _socket_route_data() into its only caller, after untangling its logic.
The old function traversed the linked list of socket_nodes in a loop,
examining each node at the bottom of the loop in order to identify the next
node. It went through elaborate and confusing gyrations to avoid dereferencing
a pointer for a node that had been deleted.
A better solution is to get a pointer to the next node *before* deleting the
current one. The resulting code is simple and easy to understand.
scottmk [Thu, 22 Oct 2009 03:21:31 +0000 (03:21 +0000)]
Various cleanups in transport_session.c:
1. In init_transport(): guard against a NULL server parameter.
2. In session_free(): if the session is still open, disconnect it.
3. In session_connect(): if we open a socket but are unable to connect to Jabber, close the
socket and set the sock_id member to zero. If the socket is already open, return an error,
instead of reusing the existing socket (and trying to overlay any open Jabber session).
4. In session_connect(): guard against an invalid auth_type.
5. In session_connect(): corrected some errors in the way we calculate buffer sizes,
6. In session_disconnect(): send a disconnect message only if the socket is still open.
7. Tidied up white space and comments in various places.
8. Added doxygen-style comments to document some of the functions.
scottmk [Sun, 18 Oct 2009 02:00:18 +0000 (02:00 +0000)]
Eliminated _socket_route_data_id() as a separate function, incorporating
its contents into the end of socket_wait().
Rationale: _socket_route_data_id() was called in only a single place.
It was little more than a mildly obfuscated if test, branching to two
very different functions. Having this code fragment in a separate function
just made the logic harder to follow.
scottmk [Tue, 13 Oct 2009 01:51:34 +0000 (01:51 +0000)]
1. Moved the declaration of socket_node from the header into the
implementation file. No other source files need to be exposed
to it.
2. Contrived to avoid leaking sockets in case of error exits;
sometimes by changing the sequence of operations, sometimes by
inserting a close().
3. In socket_open_tcp_client() and socket_open_udp_client():
removed the call to bind(). Ordinarily a client socket doesn't
need to know or care what its local address is.
4. In socket_open_udp_client(): eliminated the second and third
parameters, which define the remote address. That information
wasn't going anywhare anyway. For a UDP socket, you have no
use for the remote address until you actually try to send or
receive.
5. Added doxygen-style comments to document some of the functions.
M include/opensrf/socket_bundle.h
M src/libopensrf/socket_bundle.c
scottmk [Mon, 12 Oct 2009 18:09:18 +0000 (18:09 +0000)]
1. Replace the old JSON parser (jsonParseString()) with a newer, faster one
(jsonParse()).
2. Because the new JSON parser is strict about syntax errors that the old
parser would ignore, it was necessary to parse the srfsh command line more
intelligently. Otherwise it would be impossible to build a JSON string
internally with reliably correct syntax.
Hence: instead of using strtok() to break up the command line into tokens, we
now use a rudimentary recursive descent parser to isolate JSON strings as
distinct arguments, even if they contain embedded white space.
As a fairly benign side effect of these changes, the treatment of commas
in the command line is changing a bit. All parts of the command line
(not just JSON strings) may now be optionally separated by commas.
3. We now use an osrfStringArray to hold the results of parsing, rather
than a bare array of pointers.
4. The function formerly known as "parse_request" is now named "process_request",
because it does a lot more than just parsing.
scottmk [Sat, 10 Oct 2009 02:28:49 +0000 (02:28 +0000)]
Performance tweak to the logging routines.
_osrfLogToFile() is no longer a variadic function. We always sent it the
same format string, and there was no point in sending a variable-length
parameter list that never actually varied in length.
Now we send it a fixed-length parameter list -- and thereby avoid two
calls to vsnprintf() that had been hidden in the VA_LIST_TO_STRING macro.
Also: added doxygen-style comments to several of the functions.
scottmk [Thu, 8 Oct 2009 18:26:37 +0000 (18:26 +0000)]
Fix a bug in md5sum() (which only affected code compiled
with debugging turned on).
md5sum() builds an md5 message digest in a buffer. Originally
it used memset() to initialize the buffer with binary zeroes.
At some point the call to memset() was replaced with the
osrf_clearbuf() macro. When compiled in debugging mode,
osrf_clearbuf() fills the buffer with exclamation points;
otherwise it reverts to the original memset().
In this case the use of osrf_clearbuf is inappropriate, because
we use strcat() to build the message digest, two bytes at a
time. We don't need to use memset(), but the first byte needs
to be initialized to a nul byte so that strcat() will work as
intended. Hence:
1. Remove the call to osrf_clearbuf().
2. Put a nul byte at the beginning of the buffer.
Also, I made the buffer smaller. There's no reason for it
to be 256 bytes long.
This new version copies the original jsonObject directly, with no round
trip through a JSON string. We use jsonObjectDecodeClass() to do the
copy, in order to make sure that all class hints are decoded into
classnames.
(Until recently, jsonObjectDecodeClass() would remove classnames that were
already present. That's presumably the reason for the curious and
inefficient two-step procedure that we were using up till now.)
Also: In two locations (in osrf_message_add_object_param() and
osrf_message_add_param()) We used jsonParseString() to parse a
hard-coded "[]". I replaced those calls with equivalent (and faster)
calls to jsonNewObjectType( JSON_ARRAY );
Also: in osrf_message_set_result_content() I eliminated a pointless test
for nullity of the json_string variable. An earlier test already
guarantees that json_string is not NULL.
scottmk [Mon, 5 Oct 2009 14:48:24 +0000 (14:48 +0000)]
Created a new function osrfHashExtract(). It extracts an item with a
given key from an osrfHash, without destroying it, leaving the rest
of the osrfHash intact.
M include/opensrf/osrf_hash.h
M src/libopensrf/osrf_hash.c
scottmk [Mon, 5 Oct 2009 02:01:39 +0000 (02:01 +0000)]
Tweak jsonObjectDecodeClass so that it preserves classnames
that are already present. The previous version would add
new classnames, if it found them encoded, but would drop
any old ones.
At present this change will have no effect. Apart from a
couple of recursive calls, the only function that calls
jsonObjectDecodeClass() is jsonParseString(). In that case,
we pass a freshly parsed jsonObject that can't possibly
have any classnames yet.
However this change will enable us to use this function
elsewhere (specifically in osrf_message.c), resulting in
simpler and faster code.
scottmk [Sun, 4 Oct 2009 15:13:36 +0000 (15:13 +0000)]
Enhance the performance of the recursive descent JSON parser,
mainly the jsonParse() function.
1. The old version would create a jsonObject and then copy it, with
possible modifications, via a call to jsonObjectDecodeClass(), in
order to decode class hints into the classname member. Finally, it
would throw away the original jsonObject.
The copying operation is expensive, and the new version eliminates
it. When decoding is desired (which is nearly always), execution
passes through a parallel version of the get_hash() function, which
does the decoding on the fly.
In my benchmarking, the new version reduces the parsing time by
around 35 - 45 percent. It is now at least twice as fast as the
older jsonParseString() function, which uses a finite state
machine instead of recursive descent.
2. In get_number(): instead of allocating and destroying a
temporary growing_buffer, use the one available in the
Parser structure.
3. In osrf_json.h: Applied some pedantic corrections to the
doxygen comments for the old parser.
--------
In all cases tested, the new version produces results identical
to those of the old version. The results are also identical to
those of the older parser, apart from error detection.
M include/opensrf/osrf_json.h
M src/libopensrf/osrf_parse_json.c
We routinely grab a chunk of configuration file that doesn't
apply to the router process. When we detect the spurious hit,
we throw it away and go on to the next one.
We had been detecting the spurious hit after forking, thus
wasting a fork.
Now we detect the spurious hit before forking, so that we
don't waste a fork.
Comment out an error message added in the previous patch.
The error message reported what looks like an error condition:
the config file doesn't provide transport info. However what
appears to be happening is that the program tries to read
several different sections of the config file, of which some
are relevant and some aren't, due to some overloading of the
<routers> tag. As a result it routinely spawns irrelevant
children, only to watch them die.
There's got to be a better way, but for now I'll just
suppress the error message.
1. Eliminated some memory leaks by replacing calls to
jsonObjectFindPath() (whose return values weren't getting
freed) with calls to jsonObjectGetKey(). This change will
also speed up the configuration a bit by avoiding the
cloning of objects.
2. Changed setupRouter so that it returns void instead of int.
We weren't looking at the return value anyway. Since the
function normally enters an infinite loop, any return
represents some kind of error.
3. If the configuration file has no transport information,
issue a message before exiting (to standard error, since we
don't have a log file yet).
1. Changed osrfLogFacilityToInt() so that it accepts
a const pointer.
2. Added the const qualifier to various variables.
3. In osrf_router_main.c: Removed three inappropriate
calls to free(). Some memory leaks remain, where we
fetch some cloned jsonObjects from jsonObjectFindPath()
and don't free them.
M include/opensrf/log.h
M src/router/osrf_router_main.c
M src/gateway/osrf_http_translator.c
M src/libopensrf/osrf_prefork.c
M src/libopensrf/log.c
M src/libopensrf/osrf_application.c
2. In jsonNewObjectType(): explicitly initialize a JSON_BOOL to false, instead
of implicitly relying on the expectation that a NULL pointer is represented
by all-bits-zero.
3. In jsonObjectExtractIndex(): set the parent pointer to NULL in the
extracted jsonObject.
M include/opensrf/osrf_json.h
M src/libopensrf/osrf_json_object.c
Make building & installing the core components of OpenSRF optional.
For now, this will be of most interest to those interested in installing only
the JavaScript files from OpenSRF. If you disable the core components in
configure, then dependency checking for those core components is skipped and
the JavaScript files will be installed to the desired location.
For example:
./autogen.sh
./configure --prefix=/openils --sysconfdir=/openils/conf --disable-core
sudo make install
... will result in only the OpenSRF JavaScript files being installed in
/openils/lib/javascript/ (although at the moment, the other directories
will still be created).
In apacheGetParamValues() and apacheGetFirstParamValue():
applied const qualifier to nkey.
Also: changed the ensuing NULL tests to look at nkey
instead of key, since key has already been verified as
non-NULL.
Actually it looks like nkey is guaranteed to be non-NULL
as well, but that depends on the correct functioning
of osrfStringArray; so I left the NULL test, as
revised, in place.
In osrfNewHash(): specify a size for the osrfList used as a hash table,
so as to avoid wasting memory.
In osrfHashSet(): rearranged the logic a bit for clarity; no change in
behavior.
In osrfHashIteratorNext(): added a bit of protection against a corrupted
iterator.
Throughout: added Doxygen-style comments for documentation. Removed
some comments from the header so that they wouldn't override more
complete comments from the implementation file.
1. Moved the declaration of the osrf_host_config struct out of the header.
Nothing outside of osrf_settings.c needs access to any members of this
struct.
2. Made the osrf_settings_new_host_config function static; removed its
prototype from the header.
3. Made the "config" pointer static.
4. Rearranged osrf_settings_free_host_config a bit to protect against
attempts to free the cached config twice.
Enhanced the error messages from osrf_settings_host_value() and
osrf_settings_host_value_object() to include the requested
search path. That should make it easier to identify the caller
and thereby figure out what went wrong.
1. In osrfListSetDefault(): install the standard function
free() as the default item-freeing callback function,
instead of the gossamer-thin and useless wrapper for it,
osrfListVanillaFree(). This change eliminates a layer of
function call overhead.
2. Eliminate osrfListVanillaFree() as neither used nor useful.
3. Add doxygen-style comments to document every function.
scottmk [Mon, 31 Aug 2009 03:48:26 +0000 (03:48 +0000)]
1. Add comments to be recognized by doxygen or other such
documentation generators.
2. Change the values returned by buffer_fadd(), buffer_add(),
buffer_add_n(), and buffer_add_char() to make them more
consistent. In practice we never pay any attention to the
return values anyway.
scottmk [Wed, 5 Aug 2009 22:27:04 +0000 (22:27 +0000)]
Extended the JSON_INIT_CLEAR macro to avoid segfaults.
Scenario: converting a JSON_BOOL, with a value of true, to a JSON_HASH or
JSON_ARRAY. The true value (in a union with an osrfHash* and an osrfList*)
was being interpreted as a non_NULL pointer and deferenced. Oops.
With this change, we clear the boolean value (by nullifying one of the
unioned pointers) whenever changing from a JSON_BOOL to anything else.
scottmk [Wed, 5 Aug 2009 12:53:19 +0000 (12:53 +0000)]
Performance tweak to jsonIterator.
Instead of storing a malloc'd copy of the key of a JSON_HASH entry,
just store a const pointer to the key string stored in the
internal osrfHash. That way we don't have to do a malloc and
free every time we bump the iterator.
This change also requires the addition of a couple of const qualifiers
in the client code.
This is a performance tweak to the osrfHashGet function, a widely
used utility function.
The old version accepted a variable number of arguments: a pointer
to an osrfHash, a string optionally containing printf-style
format specifiers, and addtional parameters as needed to fill in
the blanks.
In practice, none of the code ever uses the printf-style formatting.
We always pass exactly two parameters. We burn CPU cycles scanning
the string for format specifiers and never find any.
I eliminated the unused variable parameters and turned osrfHashGet()
into a simple two-parameter function. Just in case anybody ever
wants it, I also cloned the original version into a new function
named osrfHashGetFmt, which accepts a variable number of arguments
as before.
Note that, since the signature of the function is changing,
it is necessary to recompile any open-ils programs that
call it, namely:
projects / OpenSRF.git / log