On 09/13, Steve French wrote:
Most of the changes look ok to me, although not much point in changing
the lines like:
x = 2 /* sizeof(__u16) */ ....
to
x = sizeof(__u16) ...
not sure that those particular kinds of changes help enough with
readability (but they make backports harder) - and they have 0 impact
on performance. So even if that type of change helps readability a
small amount, it could be split out from the things which could help
performance (and/or clearly improve readability).
Those kind of changes were not aimed at performance at all. IMHO it
improves readability becase a) remove a hardcoded constant, and b)
remove the (necessity of) inline comments.
I can drop those changes if you'd like.
The one area that looked confusing is this part. Can you explain why
this part of the change?
@@ -2687,20 +2683,17 @@ int smb311_posix_mkdir(const unsigned int xid,
struct inode *inode,
uni_path_len = (2 * UniStrnlen((wchar_t *)utf16_path,
PATH_MAX)) + 2;
/* MUST set path len (NameLength) to 0 opening root of share */
req->NameLength = cpu_to_le16(uni_path_len - 2);
- if (uni_path_len % 8 != 0) {
- copy_size = roundup(uni_path_len, 8);
- copy_path = kzalloc(copy_size, GFP_KERNEL);
- if (!copy_path) {
- rc = -ENOMEM;
- goto err_free_req;
- }
- memcpy((char *)copy_path, (const char *)utf16_path,
- uni_path_len);
- uni_path_len = copy_size;
- /* free before overwriting resource */
- kfree(utf16_path);
- utf16_path = copy_path;
+ copy_size = round_up(uni_path_len, 8);
+ copy_path = kzalloc(copy_size, GFP_KERNEL);
+ if (!copy_path) {
+ rc = -ENOMEM;
+ goto err_free_req;
}
+ memcpy((char *)copy_path, (const char *)utf16_path,
uni_path_len);
+ uni_path_len = copy_size;
+ /* free before overwriting resource */
+ kfree(utf16_path);
+ utf16_path = copy_path;
}
This change only removes the check "if (uni_path_len %8 != 0)" because
uni_path_len will be aligned by round_up() (if unaliged), or a no-op if
already aligned; hence no need to check it. Unless I missed something?
Cheers,
Enzo
