Surprising use of basename in finish_log_named

Surprising use of basename in finish_log_named

[Erwan Velu]

Hi Jens,

I had a input of the fio irc channel (yes some at getting into it 
sometimes :p), and he had a very interesting point.

If you specify a full path name for the iops_log, the file will stay 
into the current dir.
While looking at the code I found that finish_log_named() is performing 
the following:

     char file_name[256], *p;
     snprintf(file_name, sizeof(file_name), "%s_%s.log", prefix, postfix);
     p = basename(file_name);

Why do we basename the file_name ? What's the reason of shrinking the 
path this way ?

Note for the author: 256 is maybe too small for some ? :)))

Cheers,
Erwan

Re: Surprising use of basename in finish_log_named

[Jens Axboe]

On 04/01/2014 09:00 AM, Erwan Velu wrote:
> Hi Jens,
>
> I had a input of the fio irc channel (yes some at getting into it
> sometimes :p), and he had a very interesting point.
>
> If you specify a full path name for the iops_log, the file will stay
> into the current dir.
> While looking at the code I found that finish_log_named() is performing
> the following:
>
>      char file_name[256], *p;
>      snprintf(file_name, sizeof(file_name), "%s_%s.log", prefix, postfix);
>      p = basename(file_name);
>
> Why do we basename the file_name ? What's the reason of shrinking the
> path this way ?

I don't think there's a good reason for that. So feel free to send in a 
(tested) patch :-)

> Note for the author: 256 is maybe too small for some ? :)))

We can make it PATH_MAX instead, not a big deal.

-- 
Jens Axboe

Re: Surprising use of basename in finish_log_named

[Erwan Velu]

Le 01/04/2014 20:49, Jens Axboe a écrit :
> I don't think there's a good reason for that. So feel free to send in 
> a (tested) patch :-) 
Please find my branch ready for master here : 
https://github.com/enovance/fio/tree/erwan/nobasename
> We can make it PATH_MAX instead, not a big deal
I've been looking at it and fixing it in iolog is pretty easy but I have 
some fear that it will not be enough to fix it globally. I mean, there 
is many occurences of paths & strings set at 256. So I'll have a look at 
a global scale for this issue in another thread.

Erwan,

Re: Surprising use of basename in finish_log_named

[Jens Axboe]

On 2014-04-02 02:11, Erwan Velu wrote:
>
> Le 01/04/2014 20:49, Jens Axboe a �crit :
>> I don't think there's a good reason for that. So feel free to send in
>> a (tested) patch :-)
> Please find my branch ready for master here :
> https://github.com/enovance/fio/tree/erwan/nobasename

Thanks, pulled.

>> We can make it PATH_MAX instead, not a big deal
> I've been looking at it and fixing it in iolog is pretty easy but I have
> some fear that it will not be enough to fix it globally. I mean, there
> is many occurences of paths & strings set at 256. So I'll have a look at
> a global scale for this issue in another thread.

OK, sounds good. I'm pretty sure I consistently used 256 in most places. 
One problematic area might be around the server/client protocol. Maybe 
it's time to finally improve that... So don't worry too much about that 
part.

-- 
Jens Axboe