On Mon, Jul 09, 2012 at 05:21:56PM +0800, Wanlong Gao wrote:
> On 07/09/2012 08:22 AM, Dave Chinner wrote:
> > On Sun, Jul 08, 2012 at 07:36:37PM +0800, Wanlong Gao wrote:
> >> remove the mount options macro, use tokens instead.
> >>
> >> CC: Ben Myers <bpm@xxxxxxx>
> >> CC: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> >> CC: Dave Chinner <david@xxxxxxxxxxxxx>
> >> CC: Zach Brown <zab@xxxxxxxxx>
> >> Signed-off-by: Wanlong Gao <gaowanlong@xxxxxxxxxxxxxx>
> >> ---
> >
> > A "what's changed in this version" list would be handy here.
> >
> >> fs/xfs/xfs_super.c | 539 +++++++++++++++++++++++++++++++---------------------
> >> 1 file changed, 320 insertions(+), 219 deletions(-)
> >
> > ....
> >
> >> -
> >> -STATIC unsigned long
> >> -suffix_strtoul(char *s, char **endp, unsigned int base)
> >> +STATIC int
> >> +suffix_match_int(substring_t *s, int *result)
> >
> > I'm not sure ints are the best unit to use here....
> >
> >> {
> >> - int last, shift_left_factor = 0;
> >> - char *value = s;
> >> + int ret;
> >> + int last, shift_left_factor = 0;
> >> + char *value = s->to - 1;
> >>
> >> - last = strlen(value) - 1;
> >> - if (value[last] == 'K' || value[last] == 'k') {
> >> + if (*value == 'K' || *value == 'k') {
> >> shift_left_factor = 10;
> >> - value[last] = '\0';
> >> + s->to--;
> >> }
> >> - if (value[last] == 'M' || value[last] == 'm') {
> >> + if (*value == 'M' || *value == 'm') {
> >> shift_left_factor = 20;
> >> - value[last] = '\0';
> >> + s->to--;
> >> }
> >> - if (value[last] == 'G' || value[last] == 'g') {
> >> + if (*value == 'G' || *value == 'g') {
> >> shift_left_factor = 30;
> >> - value[last] = '\0';
> >> + s->to--;
> >> }
> >>
> >> - return simple_strtoul((const char *)s, endp, base) << shift_left_factor;
> >> + ret = match_number(s, result, 0);
> >> + *result = *result << shift_left_factor;
> >
> > Because this overflows or gives the negative values for numbers like
> > 2G far too easily. I think this function needs to return an unsigned
> > long.
>
> Do you mean the "result" should be "unsigned long" but not the return value?
> Because the return value is a error state.
result.
BTW: *result <<= shift_left_factor;
Cheers,
Dave.
--
Dave Chinner
david@xxxxxxxxxxxxx
_______________________________________________
xfs mailing list
xfs@xxxxxxxxxxx
http://oss.sgi.com/mailman/listinfo/xfs
