On 20 June 2018 at 11:51, Pekka Paalanen <ppaala...@gmail.com> wrote: > On Tue, 19 Jun 2018 18:30:13 +0100 > Emil Velikov <emil.l.veli...@gmail.com> wrote: > >> Hi Pekka, >> >> On 18 June 2018 at 15:40, Pekka Paalanen <ppaala...@gmail.com> wrote: >> > From: Daniel Stone <dani...@collabora.com> >> > >> > Rather than having a hardcoded dependency on the build-directory layout, >> > use an explicit module-map environment variable, which rewrites requests >> > for modules and helper/libexec binaries to specific paths. >> > >> > This will help with migration to Meson where setting up the paths >> > according to autotools would be painful and unnecessary. >> > >> Surely one didn't need a build system, to make nice improvements. > > The driving force behind a change should be stated in the commit > message. Granted, this paragraph was added by me, Daniel wrote only the > first paragraph. I didn't see the first paragraph as sufficient > rationale. The patch also allows loading external backends which we > specifically do not want to support at the time. > Right, this extra information seems a lot better selling point than build system A/B.
>> I guess, sometimes, tools do need to give us a wake-up call. >> >> >> > +WL_EXPORT size_t >> > +weston_module_path_from_env(const char *name, char *path, size_t path_len) >> > +{ >> > + const char *mapping = getenv("WESTON_MODULE_MAP"); >> > + const char *end; >> > + >> strlen(name) once and use it throughout? >> > > Might as well, since 'name' doesn't change. > >> >> > + if (!mapping) >> > + return 0; >> > + >> > + end = mapping + strlen(mapping); >> > + while (mapping < end && *mapping) { >> > + const char *filename, *next; >> > + >> > + /* early out: impossibly short string */ >> > + if ((size_t)(end - mapping) < strlen(name) + 1) >> > + return 0; >> > + >> > + filename = &mapping[strlen(name) + 1]; >> > + next = strchrnul(mapping, ';'); >> > + >> > + if (strncmp(mapping, name, strlen(name)) == 0 && >> > + mapping[strlen(name)] == '=') { >> > + size_t file_len = next - filename; /* no trailing >> > NUL */ >> > + if (file_len >= path_len) >> > + return 0; >> > + strncpy(path, filename, file_len); >> > + path[file_len] = '\0'; >> A simple stat() can, more or less, ensure that we're not giving random >> rubbish to the caller. >> Worth it, or more of an overkill? > > If a mapping exists, we need to return the mapped-to path. If we return > nothing, then the caller will use the default path and a broken module > map could go unnoticed if there actually is something in the default > path. > Hmm good point - shout early and loud is a good thing to have. The transient fallbacks tend to do more harm than good. With the "allow external modules" in the commit message + const [int,ssize_t] foo = strlen(name), the patch is Reviewed-by: Emil Velikov <emil.veli...@collabora.com> -Emil _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel