[ruby-core:33120] Re: [Ruby 1.9-Feature#4038] IO#advise

Hi

2010/11/10 Run Paint Run Run <[e-mail]>:
> Issue #4038 has been updated by Run Paint Run Run.
>
> File io-advise-3.patch added
>
>> I like this patch and I've reviewed this.
>
> Thank you. I've updated the documentation according to your suggestions. Does it look OK?

Looks good. :)


>> Don't we need following?
>>
>> if (!advice)
>> eturn Qnil;
>>
>> Or, should we raise not-implement exception?
>
> My thinking was that in the bizarre case where none of the POSIX_FADV_* constants were defined, but HAVE_POSIX_FADVISE was, it was acceptable to pass posix_fadvise() 0 for the advice argument. On my system, at least, POSIX_FADV_NORMAL has the value 0, so this makes even more sense. On other systems, posix_fadvise() would presumably return EINVAL in this case, which we would then raise. If this isn't acceptable, perhaps we initialise _advice_ to a sentinel value, then return Qnil if it has this value after the else statement? I'd rather not raise NotImplementedError because otherwise we try to fail silently on platforms without this syscall.

Ah ok, I missed POSIX_FADV_NORMAL=0 case. So, I don't think initialize
advise=0 is good idea. It mean initialize platform dependent meanings.
And personally I prefer that unsupported hint behave as no-op. But,
I'm ok other behavior too if unsupported
hint keeps platform independent meanings.


> Are there any security issues we need to consider? $SAFE, tainting, trust? #advise already raises a SecurityError when $SAFE=4.

I think we don't need additional limit. fadvise() is a hint of
read/write operation. and we allow read/write on $SAFE=1/2/3.

Thanks.