Re: including a new gnulib module

[John W. Eaton]

On 26-Jul-2012, c. wrote:

| > Here is a new version of the changeset that adds the new functions in 
data.cc (and links fine)
| > I can't push it at the moment as the connection to the mercurial repo at 
savannah
| > appears to be down.
| > 
| > BTW, is there a better way than copying data as I did in this 
implementation to create 
| > an Array<double> from double[] ?

If you can compute the length of the array separate from allocating
and filling it, then you could do something like

  octave_idx_type needed_length = ...;
  Array<double> buffer (needed_length);
  function_that_fills_buffer (buffer.fortran_vec ());

| +extern "C"
| +{
| +#include <base64.h>
| +}

We should probably ask the gnulib maintainers to add extern "C" to the
base64.h header file.

| +      Array<double> in = args(0).array_value ();

This should probably be const Array<double> since you aren't modifying
it.

| +      if (! error_state)
| +        {      
| +          char* inc = (char*) in.fortran_vec ();

If you don't plan to modify the IN vector, then use data () instead of
fortran_vec.  That way you won't force an unnecessary copy if there is
more than one reference to the data in the input argument to
base64_encode.

In Octave code, we prefer to avoid casts if possible, but if they are
necessary, then we prefer to use C++-style casts because they are
easier to find.

| +          size_t inlen = in.numel () * sizeof (double) / sizeof (char);
| +          
| +          char* out;
| +          
| +          size_t outlen = base64_encode_alloc (inc, inlen, &out);
| +          if (out == NULL && outlen == 0 && inlen != 0)

In C++, it's almost never necessary to use NULL.  0 usually works
fine.  Or write "!out" instead of "out == 0".

| +            error ("base64_encode: input array too large.");
| +          else if (out == NULL)
| +            error ("base64_encode: memory allocation error.");

For consistency with other messages in Octave, we don't end error
messages in periods.

| +          std::string s (out);
| +          retval(0) = octave_value (s);

You should be able to avoid creating the std::string object here.
There is an

  octave_value (const char *s, char type = '\'');

constructor.  The type says whether it is a single- or double-quoted
string.  I would write

  retval(0) = octave_value (out);

to use the default and construct a single-quoted string here.

Also, there is an octave_value_list constructor that converts a single
octave_value object to an octave_value_list object with one element,
so you could write

  return octave_value (out);

Finally, the error function simply returns, so lines that follow are
still executed.  Is that OK to do here, or should you be returning
early?  For example:

          if (! out && outlen == 0 && inlen != 0)
            {
              error ("base64_encode: input array too large");
              return retval;
            }
          else if (! out)
            {
              error ("base64_encode: memory allocation error");
              return retval;
            }

or  

          if (! out && outlen == 0 && inlen != 0)
            error ("base64_encode: input array too large");
          else if (! out)
            error ("base64_encode: memory allocation error");

          if (error_state)
            return retval;

jwe