Re: [PATCH 12/18] hw/nvram/fw_cfg: Inline fw_cfg_file_slots()

2025-06-15 Thread Philippe Mathieu-Daudé

Hi Igor,

On 6/6/25 15:29, Igor Mammedov wrote:

On Thu,  1 May 2025 23:04:50 +0200
Philippe Mathieu-Daudé  wrote:


Now than fw_cfg_file_slots() only returns
FW_CFG_FILE_SLOTS_DFLT, we can inline it.

Signed-off-by: Philippe Mathieu-Daudé 


does this even compile?
I see more usages of fw_cfg_file_slots(), then being removed here.

git grep "fw_cfg_file_slots("
hw/nvram/fw_cfg.c:static inline uint16_t fw_cfg_file_slots(const FWCfgState *s)
hw/nvram/fw_cfg.c:return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s);
hw/nvram/fw_cfg.c:dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * 
fw_cfg_file_slots(s);
hw/nvram/fw_cfg.c:assert(count < fw_cfg_file_slots(s));
hw/nvram/fw_cfg.c:assert(index < fw_cfg_file_slots(s));
hw/nvram/fw_cfg.c:if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) {
hw/nvram/fw_cfg.c: * configuration is (FW_CFG_FILE_FIRST + 
fw_cfg_file_slots(s)). */
hw/nvram/fw_cfg.c:if (fw_cfg_file_slots(s) > file_slots_max) {


No problem in the branch I posted, commit from April (and yes,
it does compile fine):

$ git grep "fw_cfg_file_slots(" 09deca844303fde6761
$

I'm OK to rebase/respin once the prerequisite series are merged.

Regards,

Phil.



if we decide to remove x-file-slots, this patch probably needs some fixing

---
  hw/nvram/fw_cfg.c | 13 -
  1 file changed, 4 insertions(+), 9 deletions(-)





Re: [PATCH 12/18] hw/nvram/fw_cfg: Inline fw_cfg_file_slots()

2025-06-06 Thread Igor Mammedov
On Thu,  1 May 2025 23:04:50 +0200
Philippe Mathieu-Daudé  wrote:

> Now than fw_cfg_file_slots() only returns
> FW_CFG_FILE_SLOTS_DFLT, we can inline it.
> 
> Signed-off-by: Philippe Mathieu-Daudé 

does this even compile?
I see more usages of fw_cfg_file_slots(), then being removed here.

git grep "fw_cfg_file_slots("
hw/nvram/fw_cfg.c:static inline uint16_t fw_cfg_file_slots(const FWCfgState *s)
hw/nvram/fw_cfg.c:return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s);
hw/nvram/fw_cfg.c:dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * 
fw_cfg_file_slots(s);
hw/nvram/fw_cfg.c:assert(count < fw_cfg_file_slots(s));
hw/nvram/fw_cfg.c:assert(index < fw_cfg_file_slots(s));
hw/nvram/fw_cfg.c:if (fw_cfg_file_slots(s) < FW_CFG_FILE_SLOTS_MIN) {
hw/nvram/fw_cfg.c: * configuration is (FW_CFG_FILE_FIRST + 
fw_cfg_file_slots(s)). */
hw/nvram/fw_cfg.c:if (fw_cfg_file_slots(s) > file_slots_max) {

if we decide to remove x-file-slots, this patch probably needs some fixing
> ---
>  hw/nvram/fw_cfg.c | 13 -
>  1 file changed, 4 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/nvram/fw_cfg.c b/hw/nvram/fw_cfg.c
> index 71c41c979d7..de65ee8342e 100644
> --- a/hw/nvram/fw_cfg.c
> +++ b/hw/nvram/fw_cfg.c
> @@ -255,15 +255,10 @@ static void fw_cfg_write(FWCfgState *s, uint8_t value)
>  /* nothing, write support removed in QEMU v2.4+ */
>  }
>  
> -static inline uint16_t fw_cfg_file_slots(const FWCfgState *s)
> -{
> -return FW_CFG_FILE_SLOTS_DFLT;
> -}
> -
>  /* Note: this function returns an exclusive limit. */
>  static inline uint32_t fw_cfg_max_entry(const FWCfgState *s)
>  {
> -return FW_CFG_FILE_FIRST + fw_cfg_file_slots(s);
> +return FW_CFG_FILE_FIRST + FW_CFG_FILE_SLOTS_DFLT;
>  }
>  
>  static int fw_cfg_select(FWCfgState *s, uint16_t key)
> @@ -845,13 +840,13 @@ void fw_cfg_add_file_callback(FWCfgState *s,  const 
> char *filename,
>  int order = 0;
>  
>  if (!s->files) {
> -dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * fw_cfg_file_slots(s);
> +dsize = sizeof(uint32_t) + sizeof(FWCfgFile) * 
> FW_CFG_FILE_SLOTS_DFLT;
>  s->files = g_malloc0(dsize);
>  fw_cfg_add_bytes(s, FW_CFG_FILE_DIR, s->files, dsize);
>  }
>  
>  count = be32_to_cpu(s->files->count);
> -assert(count < fw_cfg_file_slots(s));
> +assert(count < FW_CFG_FILE_SLOTS_DFLT);
>  
>  /* Find the insertion point, sorting by file name. */
>  for (index = count;
> @@ -926,7 +921,7 @@ void *fw_cfg_modify_file(FWCfgState *s, const char 
> *filename,
>  }
>  }
>  
> -assert(index < fw_cfg_file_slots(s));
> +assert(index < FW_CFG_FILE_SLOTS_DFLT);
>  
>  /* add new one */
>  fw_cfg_add_file_callback(s, filename, NULL, NULL, NULL, data, len, true);