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? Raag
