On Sat, Jul 3, 2021 at 10:08 PM Nick Couchman <nick.e.couchman(a)gmail.com
wrote:
On Tue, Jun 29, 2021 at 11:52 PM Solomon Boulos
<boulos(a)google.com> wrote:
> Alright, looking through some git blame -w, I think this broke and then
> was restored in 2912fad98e. Jim had a diff in 2014 (823625c5da) to make the
> code compile, but crucially the buffer code no longer had a “max len” just
> a length.
> I think this broke the “clever” reuse of the input
buffdesc as the
> argument to GetFH (which has been there since 2012 from 878f45493e).
> The input when mapped is a 16-byte thingy (sizeof
nfs23_map_handle_t),
> and GetFH needs to expand the 16-byte mapped handle into an FH4. I assume
> the code when written had a FH4_SIZE as it’s max length, rather than the
> fh_desc buffer appearing to be trimmed to 16-bytes (that is, I think there
> was a length separate from max length at the time or something).
Digging into this a little more, I
think the following commit actually
originally broke things:
3016c4cee45635afa9c25b21c2f5b014d80d1b6d
In that commit, the "netbuf" data type used as an argument
for HandleMap_GetFH is replaced with the "gsh_buffdesc" data type. As far
as I can tell, "gsh_buffdesc" never had a "maxlen" member, so the
following
code in that function would not have compiled after that:
==
...
if(rc == HASHTABLE_SUCCESS)
{
handle_pool_entry_t *h = (handle_pool_entry_t *)buffval.pdata;
if(h->fh_len < p_out_fsal_handle->maxlen)
{
p_out_fsal_handle->len = h->fh_len;
memcpy(p_out_fsal_handle->buf, h->fh_data, h->fh_len);
memcpy(p_out_fsal_handle->addr, h->fh_data, h->fh_len);
rc = HANDLEMAP_SUCCESS;
}
...
==
After that change, there are various iterations of renaming the
"p_out_fsal_handle" parameter to other things, up until the commit that you
mentioned that fixed the compile issues
(823625c5da2373606dd4f75dce46a92be143507c), which resulted in the code
having "p_out_fsal_handle" renamed to the value actually being passed in,
and the "maxlen" member replaced with "len".
And, just another note on this - in the commit prior to 3016c4cee4, I
believe the netbuf that gets passed around is allocated in this block of
code:
==
fsal_status_t
pxy_create_handle(struct fsal_export *exp_hdl,
struct fsal_handle_desc *hdl_desc,
struct fsal_obj_handle **handle)
{
fsal_status_t st;
nfs_fh4 fh4;
fsal_attrib_list_t attr;
struct pxy_obj_handle *ph;
#ifdef _HANDLE_MAPPING
char fh_data[NFS4_FHSIZE+2];
#endif
if(!exp_hdl || !hdl_desc || !handle || (hdl_desc->len
NFS4_FHSIZE))
ReturnCode(ERR_FSAL_INVAL, 0);
fh4.nfs_fh4_val = hdl_desc->start;
fh4.nfs_fh4_len = hdl_desc->len;
#ifdef _HANDLE_MAPPING
if(hdl_desc->len == sizeof(nfs23_map_handle_t)) {
nfs23_map_handle_t *h23 =
(nfs23_map_handle_t*)hdl_desc->start;
if(h23->type == PXY_HANDLE_MAPPED) {
struct netbuf fh = {
.maxlen = sizeof(fh_data),
.buf = fh_data
};
struct pxy_handle_blob *blob;
==
From this I gather that the size of maxlen for that netbuf is the size of
the data going into the buffer; however, that member is allocated as
NFS4_FHSIZE + 2.
So, maybe a constant, something like:
#define PROXYV4_HANDLE_MAXLEN NFS4_FHSIZE+2
Or maybe there's a better way?
-Nick