From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1759291AbcAKNdH (ORCPT ); Mon, 11 Jan 2016 08:33:07 -0500 Received: from relay1.mentorg.com ([192.94.38.131]:46026 "EHLO relay1.mentorg.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755615AbcAKNdF (ORCPT ); Mon, 11 Jan 2016 08:33:05 -0500 From: Andrew Gabbasov To: "'Jan Kara'" CC: Jan Kara , References: <1450974338-22762-1-git-send-email-andrew_gabbasov@mentor.com> <1450974338-22762-8-git-send-email-andrew_gabbasov@mentor.com> <20160104132500.GC13014@quack.suse.cz> In-Reply-To: <20160104132500.GC13014@quack.suse.cz> Subject: RE: [PATCH v2 7/7] udf: Merge linux specific translation into CS0 conversion function Date: Mon, 11 Jan 2016 16:31:47 +0300 Organization: Mentor Graphics Corporation Message-ID: <000101d14c74$6c1a5f40$444f1dc0$@mentor.com> MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit X-Mailer: Microsoft Outlook 14.0 Thread-Index: AQHRPme+jf6ES8ur5EaeRB2m/Xmthp7radAAgAr9jxA= Content-Language: en-us Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Jan, > -----Original Message----- > From: Jan Kara [mailto:jack@suse.cz] > Sent: Monday, January 04, 2016 4:25 PM > To: Gabbasov, Andrew > Cc: Jan Kara; linux-kernel@vger.kernel.org > Subject: Re: [PATCH v2 7/7] udf: Merge linux specific translation into CS0 conversion function [skipped] > > + u_ch = cmp_id >> 3; > > + > > + ocu++; > > + ocu_len--; > > + > > Can we just check whether ocu_len is a multiple of u_ch here > and bail out if not? That would save us some rounding below > and also fix possible access beyond the end of the string > in case fs is corrupted. Yes, I think it can be done. I just wasn't sure if having unaligned length is allowed or not, and tried to support this case too. We can definitely return -EINVAL if ocu_len is not a multiple of u_ch. [skipped] > > - for (i = 1; (i < ocu_len) && (str_o_len < str_max_len);) { > > /* Expand OSTA compressed Unicode to Unicode */ > > - uint32_t c = ocu[i++]; > > - if (cmp_id == 16) > > + c = ocu[i++]; > > + if (u_ch > 1) > > c = (c << 8) | ocu[i++]; > > > > + if (translate) { > > + if ((c == '.') && (firstDots >= 0)) > > + firstDots++; > > + else > > + firstDots = -1; > > + > > + if (c == '/' || c == 0) { > > + if (illChar) > > + continue; > > + illChar = 1; > > + needsCRC = 1; > > + c = ILLEGAL_CHAR_MARK; > > + } else { > > + illChar = 0; > > + } > > + } > > + > > len = conv_f(c, &str_o[str_o_len], str_max_len - str_o_len); > > /* Valid character? */ > > - if (len >= 0) > > + if (len >= 0) { > > str_o_len += len; > > - else > > + } else { > > str_o[str_o_len++] = '?'; > > + needsCRC = 1; > > + } > > Umm, can you try factoring out body of this loop into a helper function > and using it both during extension parsing and full name parsing? I'll try to think about it. Although I'm not sure if it could become a little overcomplicated since that helper could require too much internals of this calling function. > Also I think that checking whether the name is '.' or '..' could be moved > just into the if (translate) below where you could just directly check it like: > if (str_o_len <= 2 && str_o[0] == '.' && > (str_o_len == 1 || str_o[1] == '.')) Yes, you are right, it will be simpler. I'll do it (and get rid of firstDots) in version 3 of the patch. I'll also make all your suggested changes with empty lines and += u_ch. Thanks. Best regards, Andrew