Hello Jean-Christophe,
On Fri, Feb 07, 2014 at 07:48:57AM +0100, Jean-Christophe PLAGNIOL-VILLARD wrote:
> On 17:40 Thu 06 Feb , Uwe Kleine-K??nig wrote:
> > +#define ntohll(val) __be64_to_cpu(val)
> > +#define htonll(val) __cpu_to_be64(val)
>
> use the cpu_to and to_cpu directly
> as if we have the standard define in any header later this will cause issues
htonll isn't that standard. I doesn't exist neither in Linux userspace
nor in Linux kernel space. I would prefer to put the macro into a global
place already now. Or still better, define {ntoh,hton}{16,32,64} which
have a more intuitive naming scheme.
> > - printk("NFS: returned filename too long: %u\n", count);
> > + printf("%s: returned a too long filename: %u\n", __func__, count);
> can we use dev_xx for message
I will check where I can get my hands on a good struct device_d*.
> > +static uint32_t *nfs_add_fh3(uint32_t *p, unsigned fh_len, const char *fh)
> > +{
> > + *p++ = htonl(fh_len);
> > +
> > + /* zero padding */
> > + if (fh_len & 3)
> > + p[fh_len / 4] = 0;
> > +
> > + memcpy(p, fh, fh_len);
> > + p += DIV_ROUND_UP(fh_len, 4);
> > + return p;
> > +}
> > +
> > +static uint32_t *nfs_add_filename(uint32_t *p,
> > + uint32_t filename_len, const char *filename)
> > +{
> > + *p++ = htonl(filename_len);
> > +
> > + /* zero padding */
> > + if (filename_len & 3)
> > + p[filename_len / 4] = 0;
> > +
> > + memcpy(p, filename, filename_len);
> > + p += DIV_ROUND_UP(filename_len, 4);
> > + return p;
> > +}
> > +
> what is the difference with the function upper?
The function and parameter names and the type of the 2nd argument.
(OK, I guess that wasn't the answer you wanted to read. Yes, you're
right, they could use the same function. Something like nfs_add_string.
(Technically fh3 isn't a string<> but an opaque<NFS3_FHSIZE>, but that
doesn't really matter here. I will think a bit about proper naming.))
> > + npriv->rootfh_len = ntohl(net_read_uint32(p++));
> > + if (npriv->rootfh_len > NFS3_FHSIZE) {
> > + printf("%s: file handle too big: %lu\n", __func__,
> > + (unsigned long)npriv->rootfh_len);
> > + return -EIO;
> really EIO?
That's a protocol error and -EIO is what is returned in other places for
protocol errors, too. Still if you have a better suggestion ...
> > - ret = rpc_lookup_req(npriv, PROG_NFS, 2);
> > + ret = rpc_lookup_req(npriv, PROG_NFS, 3);
>
> so we loose nfs2?
Right. Do you consider it a loss? I don't think it worth to implement
both side by side.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
_______________________________________________
barebox mailing list
barebox@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/barebox
