[Mesa-stable] [Mesa-dev] [PATCH] mesa: Drop PATH_MAX usage.

[Roland Mainz]

On Tue, Nov 15, 2016 at 11:05 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On Tuesday, November 15, 2016 10:28:11 PM PST Roland Mainz wrote:
>> On Tue, Nov 15, 2016 at 9:56 PM, Emil Velikov <emil.l.velikov at gmail.com> wrote:
>> > On 15 November 2016 at 20:04, Kenneth Graunke <kenneth at whitecape.org> wrote:
>> >> GNU/Hurd does not define PATH_MAX since it doesn't have such arbitrary
>> >> limitation, so this failed to compile.  Apparently glibc does not
>> >> enforce PATH_MAX restrictions anyway, so it's kind of a hoax:
>> >>
>> >> https://www.gnu.org/software/libc/manual/html_node/Limits-for-Files.html
>> >>
>> >> MSVC uses a different name (_MAX_PATH) as well, which is annoying.
>> >>
>> >> We don't really need it.  We can simply asprintf() the filenames.
>> >> If the filename exceeds an OS path limit, presumably fopen() will
>> >> fail, and we already check that.  (We actually use ralloc_asprintf
>> >> because Mesa provides that everywhere, and it doesn't look like we've
>> >> provided an implementation of GNU's asprintf() for all platforms.)
>> >>
>> >> Fixes the build on GNU/Hurd.
>> >>
>> >> Cc: "13.0" <mesa-stable at lists.freedesktop.org>
>> >> Signed-off-by: Samuel Thibault <samuel.thibault at ens-lyon.org>
>> >> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>> >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=98632
>> >> ---
>> >>  src/mesa/main/arbprogram.c | 12 ++++--------
>> >>  src/mesa/main/shaderapi.c  | 37 +++++++++++--------------------------
>> >>  2 files changed, 15 insertions(+), 34 deletions(-)
>> >>
>> >> Samuel, does this fix the build for you?
>> >>
>> >> Emil, I didn't add the Fixes: tag because this was broken long before
>> >> that patch - MESA_SHADER_DUMP_PATH/MESA_SHADER_READ_PATH have existed
>> >> for a while now.
>> >>
>> > Ack, makes sense. I've only looked at the latest instance which
>> > introduces/uses the define.
>> >
>> > The patch is spot on afaict
>> > Reviewed-by: Emil Velikov <emil.velikov at collabora.com>
>>
>> -- snip --
>> +   char *name = construct_name(stage, source, read_path);
>>     f = fopen(name, "r");
>> +   ralloc_free(name);
>>     if (!f)
>>        return NULL;
>> -- snip --
>>
>> You don't need |errno|, right ? Most variants of |*free()| don't
>> preserve the |errno| value...
>
> Good catch!  But no, we don't need errno...if the file fails to open,
> we just bail, without caring why.

OK... small comment would be nice so people won't try to "fix" this in
the future - most people will think this was an accidental mistake and
then drop code to save&&restore |errno| for no good reason.

----

Bye,
Roland

-- 
  __ .  . __
 (o.\ \/ /.o) roland.mainz at nrubsig.org
  \__\/\/__/  MPEG specialist, C&&JAVA&&Sun&&Unix programmer
  /O /==\ O\  TEL +49 641 3992797
 (;O/ \/ \O;)