NFSD: Convert magic numbers to sizeof() for encode/decode
Subsystems (2)
PATCH v2 1/1 NFSD: Convert magic numbers to sizeof() for encode/decode
v2: update for 3.17.0-rc1
--- fs/nfsd/nfs4acl.c | 2 +- fs/nfsd/nfs4callback.c | 18 +- fs/nfsd/nfs4idmap.c | 4 +- fs/nfsd/nfs4proc.c | 12 +- fs/nfsd/nfs4state.c | 10 +- fs/nfsd/nfs4xdr.c | 501 +++++++++++++++++++++++++------------------------ 6 files changed, 280 insertions(+), 267 deletions(-)
fs/nfsd/nfs4acl.c +1 -1
@@ -932,7 +932,7 @@ __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who)for (i =0 ; i <ARRAY_SIZE (s2t_map); i++) {if (s2t_map[i].type != who)continue ; - p =xdr_reserve_space (xdr, s2t_map[i].stringlen +4 ); + p =xdr_reserve_space (xdr,sizeof (__be32) + s2t_map[i].stringlen);if (!p)return nfserr_resource; p = xdr_encode_opaque(p, s2t_map[i].string,
fs/nfsd/nfs4callback.c +9 -9
@@ -118,7 +118,7 @@ static void encode_nfs_cb_opnum4(struct xdr_stream *xdr, enum nfs_cb_opnum4 op) {__be32 *p; - p =xdr_reserve_space (xdr,4 ); + p =xdr_reserve_space (xdr,sizeof (__be32)); *p =cpu_to_be32 (op); }
@@ -133,7 +133,7 @@ static void encode_nfs_fh4(struct xdr_stream *xdr, const struct knfsd_fh *fh)__be32 *p;BUG_ON (length >NFS4_FHSIZE ); - p =xdr_reserve_space (xdr,4 + length); + p =xdr_reserve_space (xdr,sizeof (__be32) + length);xdr_encode_opaque (p, &fh->fh_base, length); }
@@ -235,7 +235,7 @@ static int decode_cb_op_status(struct xdr_stream *xdr, enum nfs_opnum4 expected,__be32 *p;u32 op; - p =xdr_inline_decode (xdr,4 +4 ); + p =xdr_inline_decode (xdr,2 *sizeof (__be32));if (unlikely (p ==NULL )) goto out_overflow; op =be32_to_cpup (p++);
@@ -267,7 +267,7 @@ static void encode_cb_compound4args(struct xdr_stream *xdr, {__be32 * p; - p =xdr_reserve_space (xdr,4 +4 +4 +4 ); + p =xdr_reserve_space (xdr,4 *sizeof (__be32)); p =xdr_encode_empty_array (p);/* empty tag */ *p++ =cpu_to_be32 (hdr->minorversion); *p++ =cpu_to_be32 (hdr->ident);
@@ -300,13 +300,13 @@ static int decode_cb_compound4res(struct xdr_stream *xdr,u32 length;__be32 *p; - p =xdr_inline_decode (xdr,4 +4 ); + p =xdr_inline_decode (xdr,2 *sizeof (__be32));if (unlikely (p ==NULL )) goto out_overflow; hdr->status =be32_to_cpup (p++);/* Ignore the tag */ length =be32_to_cpup (p++); - p =xdr_inline_decode (xdr, length +4 ); + p =xdr_inline_decode (xdr,sizeof (__be32) + length);if (unlikely (p ==NULL )) goto out_overflow; hdr->nops =be32_to_cpup (p);
@@ -334,7 +334,7 @@ static void encode_cb_recall4args(struct xdr_stream *xdr,encode_nfs_cb_opnum4 (xdr,OP_CB_RECALL );encode_stateid4 (xdr, &dp->dl_stid.sc_stateid); - p =xdr_reserve_space (xdr,4 ); + p =xdr_reserve_space (xdr,sizeof (__be32)); *p++ = xdr_zero;/* truncate */ encode_nfs_fh4 (xdr, &dp->dl_stid.sc_file->fi_fhandle);
@@ -367,7 +367,7 @@ static void encode_cb_sequence4args(struct xdr_stream *xdr,encode_nfs_cb_opnum4 (xdr,OP_CB_SEQUENCE );encode_sessionid4 (xdr, session); - p =xdr_reserve_space (xdr,4 +4 +4 +4 +4 ); + p =xdr_reserve_space (xdr,5 *sizeof (__be32)); *p++ =cpu_to_be32 (session->se_cb_seq_nr);/* csa_sequenceid */ *p++ = xdr_zero;/* csa_slotid */ *p++ = xdr_zero;/* csa_highest_slotid */
@@ -413,7 +413,7 @@ static int decode_cb_sequence4resok(struct xdr_stream *xdr,* If the server returns different values for sessionID, slotID or * sequence number, the server is looney tunes. */ - p =xdr_inline_decode (xdr,NFS4_MAX_SESSIONID_LEN +4 +4 +4 +4 ); + p =xdr_inline_decode (xdr,NFS4_MAX_SESSIONID_LEN +4 *sizeof (__be32));if (unlikely (p ==NULL )) goto out_overflow;memcpy (id.data, p,NFS4_MAX_SESSIONID_LEN );
fs/nfsd/nfs4idmap.c +2 -2
@@ -558,7 +558,7 @@ static __be32 encode_ascii_id(struct xdr_stream *xdr, u32 id)__be32 *p; len =sprintf (buf,"%u" , id); - p =xdr_reserve_space (xdr, len +4 ); + p =xdr_reserve_space (xdr,sizeof (__be32) + len);if (!p)return nfserr_resource; p =xdr_encode_opaque (p, buf, len);
@@ -584,7 +584,7 @@ static __be32 idmap_id_to_name(struct xdr_stream *xdr,return nfserrno (ret); ret =strlen (item->name);WARN_ON_ONCE (ret >IDMAP_NAMESZ ); - p =xdr_reserve_space (xdr, ret +4 ); + p =xdr_reserve_space (xdr,sizeof (__be32) + ret);if (!p)return nfserr_resource; p =xdr_encode_opaque (p, item->name, ret);
fs/nfsd/nfs4proc.c +6 -6
@@ -1276,7 +1276,7 @@ nfsd4_proc_compound(struct svc_rqst *rqstp,svcxdr_init_encode (rqstp, resp); resp->tagp = resp->xdr.p;/* reserve space for: taglen, tag, and opcnt */ -xdr_reserve_space (&resp->xdr,8 + args->taglen); +xdr_reserve_space (&resp->xdr,2 *sizeof (__be32) + args->taglen); resp->taglen = args->taglen; resp->tag = args->tag; resp->rqstp = rqstp;
@@ -1465,19 +1465,19 @@ static inline u32 nfsd4_getattr_rsize(struct svc_rqst *rqstp,return svc_max_payload (rqstp);if (bmap1 &FATTR4_WORD1_OWNER ) { - ret +=IDMAP_NAMESZ +4 ; + ret +=IDMAP_NAMESZ +sizeof (__be32); bmap1 &= ~FATTR4_WORD1_OWNER ; }if (bmap1 &FATTR4_WORD1_OWNER_GROUP ) { - ret +=IDMAP_NAMESZ +4 ; + ret +=IDMAP_NAMESZ +sizeof (__be32); bmap1 &= ~FATTR4_WORD1_OWNER_GROUP ; }if (bmap0 &FATTR4_WORD0_FILEHANDLE ) { - ret +=NFS4_FHSIZE +4 ; + ret +=NFS4_FHSIZE +sizeof (__be32); bmap0 &= ~FATTR4_WORD0_FILEHANDLE ; }if (bmap2 &FATTR4_WORD2_SECURITY_LABEL ) { - ret +=NFSD4_MAX_SEC_LABEL_LEN +12 ; + ret +=NFSD4_MAX_SEC_LABEL_LEN +3 *sizeof (__be32); bmap2 &= ~FATTR4_WORD2_SECURITY_LABEL ; }/*
@@ -1486,7 +1486,7 @@ static inline u32 nfsd4_getattr_rsize(struct svc_rqst *rqstp,*/ ret +=16 * (hweight32 (bmap0) +hweight32 (bmap1) +hweight32 (bmap2));/* bitmask, length */ - ret +=20 ; + ret +=5 *sizeof (__be32);return ret; }
fs/nfsd/nfs4state.c +9 -1
@@ -1215,7 +1215,15 @@ gen_sessionid(struct nfsd4_session *ses)* verifier), 12 for the compound header (with zero-length tag), and 44 * for the SEQUENCE op response: */ -#define NFSD_MIN_HDR_SEQ_SZ (24 + 12 + 44) +#define NFSD_MIN_HDR_SEQ_SZ (/* Xid, Message Type, Reply State, Flavor, +* Flavor Length, Accept State */ \ + 6 * sizeof(__be32) + \ + /* Status, Tag Length, Operation Count */ \ + 3 * sizeof(__be32) + \ + /* Operation Code, Status, SessionID, SequenceID, + * SlotID, Highest SlotID, Target Highest SlotID, + * Status Flags */ \ + 7 * sizeof(__be32) + NFS4_MAX_SESSIONID_LEN)static void free_session_slots (struct nfsd4_session *ses)
fs/nfsd/nfs4xdr.c +253 -248
1,652 lines not on this page · raw message
Signature · 1 line
1.9.3
Quoted 18 lines · Kinglong Mee
v2: update for 3.17.0-rc1
Reported-by Christoph Hellwig <[email protected]>Signed-off-by Kinglong Mee <[email protected]>--- fs/nfsd/nfs4acl.c | 2 +- fs/nfsd/nfs4callback.c | 18 +- fs/nfsd/nfs4idmap.c | 4 +- fs/nfsd/nfs4proc.c | 12 +- fs/nfsd/nfs4state.c | 10 +- fs/nfsd/nfs4xdr.c | 501 +++++++++++++++++++++++++------------------------ 6 files changed, 280 insertions(+), 267 deletions(-)fs/nfsd/nfs4acl.c +0
-0
fs/nfsd/nfs4acl.c __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who) · line 935
Quoted 5 lines
fs/nfsd/nfs4acl.c +1
-1@@ -932,7 +932,7 @@ __be32 nfs4_acl_write_who(struct xdr_stream *xdr, int who)for (i =0 ; i <ARRAY_SIZE (s2t_map); i++) {if (s2t_map[i].type != who)continue ; - p =xdr_reserve_space (xdr, s2t_map[i].stringlen +4 ); + p =xdr_reserve_space (xdr,sizeof (__be32) + s2t_map[i].stringlen);
OK, can someone please tell me how this is useful for documentation purposes? Anybody who doesn't know that sizeof(__be32) == 4 has no business working on XDR code.
I could understand this kind of patch if you were converting to sizeof(<variable name>), as that documents exactly which variable you are going to encode in this buffer and so is better than a naked value, but how is sizeof(__be32) any more useful documentation than "4"?
Trond
Quoted 8 lines · Trond Myklebust
OK, can someone please tell me how this is useful for documentation purposes? Anybody who doesn't know that
sizeof(__be32)== 4 has no business working on XDR code.
I could understand this kind of patch if you were converting to sizeof(<variable name>), as that documents exactly which variable you are going to encode in this buffer and so is better than a naked value, but how issizeof(__be32)any more useful documentation than "4"?
I think this was in reply to my complaints about the very magic length reserved. I think arithmetic expressions in the form of 4 + 4 + 8 etc are fine too, although the sizeof expressions seem to be even more obvious and thus preferable to me. The main problem is something like:
p = xdr_reserve_space(xdr, 20);
hidden deep down in a routine which requires you to count over all kinds of statements working with the XDR stream.
Quoted 16 lines · Christoph Hellwig
Quoted 8 lines · Trond Myklebust
OK, can someone please tell me how this is useful for documentation purposes? Anybody who doesn't know that
sizeof(__be32)== 4 has no business working on XDR code.
I could understand this kind of patch if you were converting to sizeof(<variable name>), as that documents exactly which variable you are going to encode in this buffer and so is better than a naked value, but how issizeof(__be32)any more useful documentation than "4"?I think this was in reply to my complaints about the very magic length reserved. I think arithmetic expressions in the form of 4 + 4 + 8 etc are fine too, although the sizeof expressions seem to be even more obvious and thus preferable to me. The main problem is something like:
Quoted 4 lines
p = xdr_reserve_space(xdr, 20);hidden deep down in a routine which requires you to count over all kinds of statements working with the XDR stream.
OK. So maybe a better approach would be individual patches for confusing spots as we come across them.
--b.