> >
> > diff --git a/lib/spdm/req-sysfs.c b/lib/spdm/req-sysfs.c
> > index 9bbed7abc153..afba3c5a2e8f 100644
> > --- a/lib/spdm/req-sysfs.c
> > +++ b/lib/spdm/req-sysfs.c
> > @@ -93,3 +93,83 @@ const struct attribute_group spdm_attr_group = {
> > .attrs = spdm_attrs,
> > .is_visible = spdm_attrs_are_visible,
> > };
> > +
> > +/* certificates attributes */
> > +
> > +static umode_t spdm_certificates_are_visible(struct kobject *kobj,
> > + struct bin_attribute *a, int n)
> > +{
> > + struct device *dev = kobj_to_dev(kobj);
> > + struct spdm_state *spdm_state = dev_to_spdm_state(dev);
> > + u8 slot = a->attr.name[4] - '0';
>
> This is clever, but the @n parameter already conveys the index.
That's still fragile. I'd use a container structure so that we can
get the number directly from container_of() and appropriate field
in the container structure.
>
> > +
> > + if (IS_ERR_OR_NULL(spdm_state))
> > + return SYSFS_GROUP_INVISIBLE;
> > +
> > + if (!(spdm_state->supported_slots & BIT(slot)))
> > + return 0;
> > +
> > + return a->attr.mode;
> > +}
> > +
> > +static ssize_t spdm_cert_read(struct file *file, struct kobject *kobj,
> > + struct bin_attribute *a, char *buf, loff_t off,
> > + size_t count)
> > +{
> > + struct device *dev = kobj_to_dev(kobj);
> > + struct spdm_state *spdm_state = dev_to_spdm_state(dev);
> > + u8 slot = a->attr.name[4] - '0';
>
> Similar comment on cleverness, I will note that the way this is
> typically handled is something like this which is just slightly less
> error prone if someone in the future changes the naming scheme.
>
> #define CERT_ATTR(n) \
> static ssize_t slot##n##_show(struct file *file, struct kobject *kobj, \
> struct bin_attribute *a, char *buf, loff_t off, \
> size_t count) \
> { \
> return spdm_cert_read(kobj_to_dev(kobj), buf, off, count, (n)); \
> } \
> static BIN_ATTR_RO(slot##n);
Or augment the attribute by sticking it in a container structure with the slot
number as data and use container_of(). Either path works fine and avoids the
fragility issue of using the naming.
>
