Code review comment for lp:~wlxing/ecryptfs/fix-bug-1678689

Revision history for this message
Jason Xing (wlxing) wrote :

Hi Tyler,

Thanks for your check.

Some test cases without "i = st_len":
1) If you add other options like "-o key=passphrase", It would cause
the problem saying "Error attempting to evaluate mount options: [-14]
Bad address...".
2) If you add other options like "-o key=passphrase", It would cause
the problem saying "Error attempting to evaluate mount options: [-22]
Invalid argument...".

Explanation:
If you look at this line "if ((i-j) > 1) {" (it is just several lines
below the break code), you will understand that "i" here means the
length of the whole separate option(eg. key=passpharse). If I just add
break without setting i to st_len, it will skip this line "if ((i-j) >
1) {" and will not allocate any memory for "value".

Does it make sense?

Jason

On Fri, May 26, 2017 at 12:21 AM, Tyler Hicks <email address hidden> wrote:
> Review: Needs Information
>
> Hi Jason - Thanks for the fix!
>
> As you know, the code that you're patching breaks "name=value" pairs up into individual parts. The 'i' variable is used to represent the length of the "name" portion of the string.
>
> I don't think it is correct to set i equal to st_len as st_len is the length of the entire "name=value" string. IIUC, simply adding a break would fix the bug.
>
> What was your reasoning for setting i to st_len?
> --
> https://code.launchpad.net/~wlxing/ecryptfs/fix-bug-1678689/+merge/321653
> You are the owner of lp:~wlxing/ecryptfs/fix-bug-1678689.

« Back to merge proposal