On Tue, Nov 15, 2016 at 11:05 PM, Kenneth Graunke <kenn...@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.veli...@gmail.com> >> wrote: >> > On 15 November 2016 at 20:04, Kenneth Graunke <kenn...@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-sta...@lists.freedesktop.org> >> >> Signed-off-by: Samuel Thibault <samuel.thiba...@ens-lyon.org> >> >> Signed-off-by: Kenneth Graunke <kenn...@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.veli...@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.ma...@nrubsig.org \__\/\/__/ MPEG specialist, C&&JAVA&&Sun&&Unix programmer /O /==\ O\ TEL +49 641 3992797 (;O/ \/ \O;) _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
