[continued from previous message]   
      
   look for a single character in a string.  Use `strchr` instead.   
      
   Finally, once the frame has been decoded and the bits copied to   
   their respective destinations, there's this code, to move   
   whatever frame data may have been received from the network to   
   the front of the context buffer for use when receiving the next   
   frame:   
      
   -   int len = strlen(ctx->buf) + 2;   
   -   memmove(ctx->buf, ctx->buf + len, ctx->ix - len);   
   -   ctx->ix -= len;   
      
   First, the return type of `strlen` is `size_t`, not `int`, but   
   that's minor.  The real issue is that you've got an off-by-one   
   error here; `strlen` will not include the terminating NUL at the   
   end of the string, so I understand why you want to add something   
   to it's return value, but it's also true that, if `s` is a C   
   string and `l = strlen(s)`; then `s[l] == 0`, and `s+l+1` points   
   to the char immediately _after_ the terminating 0.  That is, the   
   length of the entire frame, including the NUL terminator, is   
   `strlen(ctx->buf) + 1`, not +2.  Here, you appear to be   
   discarding the first byte of the next frame, though the protocol   
   does say that a message can have any number of newlines   
   following the NUL byte, so perhaps that's what you are doing.   
   Regardles, you should validate that you're not throwing away   
   valid data.   
      
   Next up, let's look at `simple_stomp_init`:   
      
   -int simple_stomp_init(simple_stomp_t *ctx, const char *host, int port,   
   error_handler eh)`   
   -{   
   -   ....   
   -   struct sockaddr local, remote;   
   -    ....   
   -   local.sa_family = AF_INET;   
   -   memset(local.sa_data, 0, sizeof(local.sa_data));   
   -   status = bind(ctx->sd, &local, sizeof(local));   
      
   You don't need to `bind` here.  It's unclear why you do.   
      
   -   remote.sa_family = hostinfo->h_addrtype;   
   -   memcpy(remote.sa_data + 2, hostinfo->h_addr_list[0], hostinfo->h_length);   
   -   *((short *)remote.sa_data) = htons(port);   
   -   status = connect(ctx->sd, &remote, sizeof(remote));   
      
   It's totally unclear why you would abuse `struct sockaddr` like   
   this: the structure of `sa_data` is meant to be opaque.  Just   
   use `sockaddr_in` here, and as all of the available   
   documentation demonstrates.  This sequence then becomes:   
      
   struct sockaddr_in remote;   
   ...   
   memset(&remote, 0, sizeof(remote));   
   remote.sin_family = AF_INET;   
   remote.sin_port = ntohs(port);   
   memcpy(&remote.sin_addr, hostinfo->h_addr_list[0], hostinfo->h_length);   
   status = connect(ctx->sd, (struct sockaddr *)&remote, sizeof(remote));   
      
   See the aforementioned VSI TCP/IP Services manual for details   
   and examples.   
      
   Continuing on, we get to this code:   
      
   -   if(!helper_read(ctx, cmd, header, body))   
   -   {   
   -      snprintf(errmsg, sizeof(errmsg), "Error receiving CONNECTED");   
   -      ctx->eh(errmsg);   
   -      return 0;   
   -   }   
      
   Here, we are attempting to read a frame in response to a message   
   we sent the server ("CONNECT").  We deduce that the server is   
   supposed to send us a "CONNECTED" frame, but we only verify that   
   we receive _a_ frame; the contents of that frame is not   
   validated.  You should probably check it to make sure it is what   
   you expect.   
      
   Note also that we're reading into fixed sized buffers that are   
   very small (80 bytes!) and easily overflowed.   
      
   Finally with respect to initialization, this code leaks sockets   
   on init failure: after the socket has been created, and even   
   connected, if the rest of initialization fails for any reason,   
   then we don't close the socket descriptor, and there's no good   
   way for calling code to know that it's open; error handling code   
   should close it consistently.   
      
   In general, things in this module are poorly named,   
   inconsistently formatted, overly verbose, and repetitious (e.g.,   
   the `XDEBUG` stuff is easily factored out into a small helper   
   function); there's a lot of "accidentally quadratic" code that   
   does things like loop over the value of `strlen` for some   
   string.  I certainly wouldn't trust this in a production   
   setting, though it may be fine for hobbyist use or a   
   non-critical internal tool.   
      
   >* vms_simple_stomp : a wrapper exposing a VMS API (strings by   
   >   descriptor, integers always by reference)   
      
   This code does a lot of extraneous allocation and copying, to   
   convert from a string descriptor to a NUL-terminated C-style   
   string: presumably the `malloc`'ing is done to ensure there's   
   space for the 0-terminator.  But with relatively modest   
   interface changes, none of that is necessary, and indeed, a lot   
   of the copying in the core library is unnecessary as well.  The   
   trick is to define a local type for a sized string, convert the   
   descriptor to an instance of that, and then use that type   
   consistently in the core library.  One then has a function that   
   serializes a frame into a buffer, which can be sent, and one to   
   retrieve a raw frame from the context, which is then parsed into   
   a suitable representation.  Everything can then be stack   
   allocated.   
      
   Also, in the `read` cases, even though you have access to   
   destination buffer sizes and it's easy enough to plumb them   
   through, you did not pass those to the reading routines, so they   
   cannot validate their destination buffers or check for   
   truncation.   
      
   >* pstomp : a Pascal wrapper to make it easier to use in   
   >   Pascal (build to PEN)   
      
   I'm afraid that I did not look at this closely.   
      
   >There are 3 changes from 0.4 to 0.5:   
   >- The _read function is now supplemented by _sub and _readone   
   >   functions for the case where one want to read more than one   
   >   message (_read sort of work for reading multiple messages from   
   >   a queue but it totally fucks up reading multiple messages from   
   >   a topic)   
   >- the low level reading now handle the case where a single TCP recv   
   >   receives more than one message, before that would result in   
   >   second message getting lost   
   >- avoid appending a LF to messages when sending - other libraries does   
   >   not do that   
      
   See above.  On a general note, you'd be much better off tracking   
   this sort of thing in a proper revision control tool; perhaps   
   putting that code into (say) a `git` repository.   
      
   I addressed most of these issues in a change I pushed to github   
   here:  https://github.com/dancrossnyc/stompvms.git   
      
   I have no real means to test this against a network server, but   
   it compiles and the simple test exercisor I wrote for the frame   
   serdes code runs on Eisner.   
      
   	- Dan C.   
      
   PS: the reference for this little protocol:   
   https://stomp.github.io/stomp-specification-1.2.html   
      
   --- SoupGate-Win32 v1.05   
    * Origin: you cannot sedate... all the things you hate (1:229/2)