On 09:52 Fri 07 Feb , Uwe Kleine-K??nig wrote:
> 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.
yeah agreed
>
> > > - 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 ...
-EPROTO no?
>
> > > - 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.
I see this as a compatibility issue
>
> 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
