On Thu, 19 Sep 2024, Raag Jadav <raag.jadav@xxxxxxxxx> wrote: > On Thu, Sep 19, 2024 at 12:24:09PM +0300, Jani Nikula wrote: >> On Thu, 19 Sep 2024, Raag Jadav <raag.jadav@xxxxxxxxx> wrote: >> > On Thu, Sep 19, 2024 at 10:38:51AM +0300, Jani Nikula wrote: >> >> On Thu, 19 Sep 2024, Raag Jadav <raag.jadav@xxxxxxxxx> wrote: >> >> > On Tue, Sep 17, 2024 at 10:49:07AM +0300, Jani Nikula wrote: >> >> >> On Tue, 17 Sep 2024, Raag Jadav <raag.jadav@xxxxxxxxx> wrote: >> >> >> > >> >> >> > +extern const char *const wedge_recovery_opts[]; >> >> >> >> >> >> Data is not an interface. Please add a function for this. >> >> > >> >> > For a single user? >> >> >> >> Yes. >> >> >> >> Well, you kind of have two, and both places need to do bounds checking >> >> on indexing the array. You also need to do bounds checking on the string >> >> manipulation, you can't just strcat and assume it'll be all right. >> > >> > Which would be true if we were to receive an unknown string. Here we sorta >> > know it offhand so we're not gonna shoot in our foot :D >> >> The thing about long term code maintenance is that "we know" often turns >> into "not too obvious" and "probably" somewhere down the line, as >> features get added and code gets refactored and moved about. >> >> Here, it only takes a new, longer string, and failure to manually check >> that the lengths don't exceed the magic 32 bytes. Just be safe from the >> start, and you don't have to worry about it later. > > On that note... > >> > Anyway, would you prefer strlcat instead? >> >> I think the cleaner option is: >> >> char event_string[32]; >> >> snprintf(event_string, sizeof(event_string), "WEDGED=%s", wedge_name(method)); >> >> which is also what most other code constructing environments for >> kobject_uevent_env() do. > > ...should we use kasprintf instead of hardcoding size? You can if you want. > > Raag -- Jani Nikula, Intel
