Re: [PATCH 3/7] disk: define nullified functions for !PARTITIONS

[AKASHI Takahiro <takahiro.akashi@linaro.org>]

Hi Tom,

On Tue, Apr 19, 2022 at 08:12:07AM -0400, Tom Rini wrote:
> On Tue, Apr 19, 2022 at 01:11:23PM +0900, AKASHI Takahiro wrote:
> > On Mon, Apr 18, 2022 at 11:09:38PM -0400, Tom Rini wrote:
> > > On Tue, Apr 19, 2022 at 10:01:54AM +0900, AKASHI Takahiro wrote:
> > > 
> > > > Some defconfig enables CMD_PART even if none of any partition table
> > > > types (CONFIG_*_PARTITION) are enabled.
> > > > This will lead to the size growth in SPL/TPL code since disk/part.c
> > > > will be compiled in any way.
> > > > We will change disk/Kconfig later so that CONFIG_PARTITIONS is only
> > > > enabled when, at least, one of CONFIG_*_PARTITION is enabled.
> > > > 
> > > > To make the build work (in particular, "part" command) correctly,
> > > > a few functions should be defined as void functions in case of
> > > > !CONFIG_PARTITIONS.
> > > > 
> > > > Signed-off-by: AKASHI Takahiro <takahiro.akashi@linaro.org>
> > > 
> > > I guess I wonder why we don't just make CMD_PART depend on PARTITIONS
> > > now and thus correct the few (single?) board that has this enabled
> > > without underlying partition code by removing the can't be functional
> > > cmd.
> > 
> > Well, that is partially what I did in my RFC and I thought
> > that you declined to accept my change.
> > Did I misunderstand you?
> 
> Yes, I wasn't clear, sorry for the confusion.  Just this part of the
> series should be replaced with making CMD_PART depend on PARTITIONS and
> if there really is a use case for 'part' without PARTITION support
> (rather than it being an unintentionally enabled feature) we can deal
> with it then.  The rest of the series looks good to me and I'll let
> Heinrich comment on the EFI specific parts.

I do understand what you expect here, but, what I call, "nullified
function" technique is already used in several places.
For instance, take blk_get_device_part_str() function which has
a nullified version of definition in include/part.h.
It is used without explicit dependency on CONFIG_PARTITIONS at:
        cmd/zfs.c
        cmd/disk.c
        cmd/reiser.c
        cmd/fat.c
        env/ext4.c
        env/fat.c

So I would like to propose to create another patch that you expect (and
what I did in RFC) instead of removing this patch because the latter
has no negative effect.

If you agree, I will post it as a separate patch.


-Takahiro Akashi

> -- 
> Tom