[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: including a new gnulib module
From: |
John W. Eaton |
Subject: |
Re: including a new gnulib module |
Date: |
Thu, 26 Jul 2012 09:42:09 -0400 |
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
- Re: including a new gnulib module, (continued)
Re: including a new gnulib module, c., 2012/07/24
- Re: including a new gnulib module, John W. Eaton, 2012/07/24
- Re: including a new gnulib module, c., 2012/07/26
- Re: including a new gnulib module, c., 2012/07/26
- Re: including a new gnulib module, Max Brister, 2012/07/26
- Re: including a new gnulib module, c., 2012/07/26
- Re: including a new gnulib module, Max Brister, 2012/07/26
- Re: including a new gnulib module, John W. Eaton, 2012/07/26
Re: including a new gnulib module,
John W. Eaton <=
Re: including a new gnulib module, c., 2012/07/29
Re: including a new gnulib module, Ben Abbott, 2012/07/30
Re: including a new gnulib module, Ben Abbott, 2012/07/30
Re: including a new gnulib module, John W. Eaton, 2012/07/31
Re: including a new gnulib module, Ben Abbott, 2012/07/31
Re: including a new gnulib module, Jordi GutiƩrrez Hermoso, 2012/07/31