linux-mtd.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [bug report] mtd: parsers: add support for Sercomm partitions
@ 2022-05-18 17:33 Dan Carpenter
  2022-05-28 10:49 ` Mikhail Zhilkin
  0 siblings, 1 reply; 2+ messages in thread
From: Dan Carpenter @ 2022-05-18 17:33 UTC (permalink / raw)
  To: csharper2005; +Cc: linux-mtd

Hello Mikhail Zhilkin,

The patch 4213e556fe2a: "mtd: parsers: add support for Sercomm
partitions" from May 3, 2022, leads to the following Smatch static
checker warning:

	drivers/mtd/parsers/scpart.c:155 scpart_parse()
	warn: missing error code here? 'mtd_get_of_node()' failed. 'res' = '0'

drivers/mtd/parsers/scpart.c
    138 static int scpart_parse(struct mtd_info *master,
    139                         const struct mtd_partition **pparts,
    140                         struct mtd_part_parser_data *data)
    141 {
    142         struct sc_part_desc *scpart_map = NULL;
    143         struct mtd_partition *parts = NULL;
    144         struct device_node *mtd_node;
    145         struct device_node *ofpart_node;
    146         struct device_node *pp;
    147         const char *partname;
    148         int nr_scparts;
    149         int nr_parts = 0;
    150         int n;
    151         int res = 0;
    152 
    153         mtd_node = mtd_get_of_node(master);
    154         if (!mtd_node)
--> 155                 goto out;
                        ^^^^^^^^

    156 
    157         ofpart_node = of_get_child_by_name(mtd_node, "partitions");
    158         if (!ofpart_node)
    159                 goto out;
                        ^^^^^^^^
Are these supposed to be success paths?

    160 
    161         nr_scparts = scpart_find_partmap(master, &scpart_map);
    162         if (nr_scparts <= 0) {

Is nr_scparts == 0 a success path?

    163                 res = nr_scparts;
    164                 goto free;
    165         }
    166 
    167         parts = kcalloc(of_get_child_count(ofpart_node), sizeof(*parts),
    168                 GFP_KERNEL);
    169         if (!parts) {
    170                 res = -ENOMEM;
    171                 goto out;
    172         }
    173 
    174         for_each_child_of_node(ofpart_node, pp) {
    175                 u32 scpart_id;
    176 
    177                 if (of_property_read_u32(pp, "sercomm,scpart-id", &scpart_id))
    178                         continue;
    179 
    180                 for (n = 0 ; n < nr_scparts ; n++)
    181                         if ((scpart_map[n].part_id != ID_ALREADY_FOUND) &&
    182                                         (scpart_id == scpart_map[n].part_id))
    183                                 break;
    184                 if (n >= nr_scparts)
    185                         /* not match */
    186                         continue;
    187 
    188                 /* add the partition found in OF into MTD partition array */
    189                 parts[nr_parts].offset = scpart_map[n].part_offs;
    190                 parts[nr_parts].size = scpart_map[n].part_bytes;
    191                 parts[nr_parts].of_node = pp;
    192 
    193                 if (!of_property_read_string(pp, "label", &partname))
    194                         parts[nr_parts].name = partname;
    195                 if (of_property_read_bool(pp, "read-only"))
    196                         parts[nr_parts].mask_flags |= MTD_WRITEABLE;
    197                 if (of_property_read_bool(pp, "lock"))
    198                         parts[nr_parts].mask_flags |= MTD_POWERUP_LOCK;
    199 
    200                 /* mark as 'done' */
    201                 scpart_map[n].part_id = ID_ALREADY_FOUND;
    202 
    203                 nr_parts++;
    204         }
    205 
    206         if (nr_parts > 0) {
    207                 *pparts = parts;
    208                 res = nr_parts;
    209         } else
    210                 pr_info("No partition in OF matches partition ID with 'SC PART MAP'.\n");
    211 
    212         of_node_put(pp);
    213 
    214 free:
    215         kfree(scpart_map);
    216         if (res <= 0)
    217                 kfree(parts);
    218 
    219 out:
    220         return res;
    221 }

regards,
dan carpenter

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [bug report] mtd: parsers: add support for Sercomm partitions
  2022-05-18 17:33 [bug report] mtd: parsers: add support for Sercomm partitions Dan Carpenter
@ 2022-05-28 10:49 ` Mikhail Zhilkin
  0 siblings, 0 replies; 2+ messages in thread
From: Mikhail Zhilkin @ 2022-05-28 10:49 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: linux-mtd

Hi Dan,

On 5/18/2022 8:33 PM, Dan Carpenter wrote:
> Hello Mikhail Zhilkin,
>
> The patch 4213e556fe2a: "mtd: parsers: add support for Sercomm
> partitions" from May 3, 2022, leads to the following Smatch static
> checker warning:
>
> 	drivers/mtd/parsers/scpart.c:155 scpart_parse()
> 	warn: missing error code here? 'mtd_get_of_node()' failed. 'res' = '0'

Thank you for your feedback! Which key do you use for checking? I can't
get these warning.

~/_utils/smatch/smatch_scripts/kchecker --pedantic
drivers/mtd/parsers/scpart.c
  CHECK   scripts/mod/empty.c
  CALL    scripts/checksyscalls.sh
  CALL    scripts/atomic/check-atomics.sh
  CC [M]  drivers/mtd/parsers/scpart.o
  CHECK   drivers/mtd/parsers/scpart.c

> drivers/mtd/parsers/scpart.c
>     138 static int scpart_parse(struct mtd_info *master,
>     139                         const struct mtd_partition **pparts,
>     140                         struct mtd_part_parser_data *data)
>     141 {
>     142         struct sc_part_desc *scpart_map = NULL;
>     143         struct mtd_partition *parts = NULL;
>     144         struct device_node *mtd_node;
>     145         struct device_node *ofpart_node;
>     146         struct device_node *pp;
>     147         const char *partname;
>     148         int nr_scparts;
>     149         int nr_parts = 0;
>     150         int n;
>     151         int res = 0;
>     152 
>     153         mtd_node = mtd_get_of_node(master);
>     154         if (!mtd_node)
> --> 155                 goto out;
>                         ^^^^^^^^
>
>     156 
>     157         ofpart_node = of_get_child_by_name(mtd_node, "partitions");
>     158         if (!ofpart_node)
>     159                 goto out;
>                         ^^^^^^^^
> Are these supposed to be success paths?

Really, I think -ENOENT is more suitable here.

    ofpart_node = of_get_child_by_name(mtd_node, "partitions");
    if (!ofpart_node)
        pr_info("%s: 'partitions' subnode not found on %pOF.\n",
                master->name, mtd_node);
        res = -ENOENT;
        goto out;

    nr_scparts = scpart_find_partmap(master, &scpart_map);
    if (nr_scparts <= 0) {
        pr_info("No any partitions was found in 'SC PART MAP'.\n");
        res = -ENOENT;
        goto free;
    }


>     160 
>     161         nr_scparts = scpart_find_partmap(master, &scpart_map);
>     162         if (nr_scparts <= 0) {
>
> Is nr_scparts == 0 a success path?
>
>     163                 res = nr_scparts;
>     164                 goto free;
>     165         }
>     166 
>     167         parts = kcalloc(of_get_child_count(ofpart_node), sizeof(*parts),
>     168                 GFP_KERNEL);
>     169         if (!parts) {
>     170                 res = -ENOMEM;
>     171                 goto out;
>     172         }
>     173 
>     174         for_each_child_of_node(ofpart_node, pp) {
>     175                 u32 scpart_id;
>     176 
>     177                 if (of_property_read_u32(pp, "sercomm,scpart-id", &scpart_id))
>     178                         continue;
>     179 
>     180                 for (n = 0 ; n < nr_scparts ; n++)
>     181                         if ((scpart_map[n].part_id != ID_ALREADY_FOUND) &&
>     182                                         (scpart_id == scpart_map[n].part_id))
>     183                                 break;
>     184                 if (n >= nr_scparts)
>     185                         /* not match */
>     186                         continue;
>     187 
>     188                 /* add the partition found in OF into MTD partition array */
>     189                 parts[nr_parts].offset = scpart_map[n].part_offs;
>     190                 parts[nr_parts].size = scpart_map[n].part_bytes;
>     191                 parts[nr_parts].of_node = pp;
>     192 
>     193                 if (!of_property_read_string(pp, "label", &partname))
>     194                         parts[nr_parts].name = partname;
>     195                 if (of_property_read_bool(pp, "read-only"))
>     196                         parts[nr_parts].mask_flags |= MTD_WRITEABLE;
>     197                 if (of_property_read_bool(pp, "lock"))
>     198                         parts[nr_parts].mask_flags |= MTD_POWERUP_LOCK;
>     199 
>     200                 /* mark as 'done' */
>     201                 scpart_map[n].part_id = ID_ALREADY_FOUND;
>     202 
>     203                 nr_parts++;
>     204         }
>     205 
>     206         if (nr_parts > 0) {
>     207                 *pparts = parts;
>     208                 res = nr_parts;
>     209         } else
>     210                 pr_info("No partition in OF matches partition ID with 'SC PART MAP'.\n");
>     211 
>     212         of_node_put(pp);
>     213 
>     214 free:
>     215         kfree(scpart_map);
>     216         if (res <= 0)
>     217                 kfree(parts);
>     218 
>     219 out:
>     220         return res;
>     221 }
>
> regards,
> dan carpenter

-- 
Best regards,
Mikhail


______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-05-28 10:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-18 17:33 [bug report] mtd: parsers: add support for Sercomm partitions Dan Carpenter
2022-05-28 10:49 ` Mikhail Zhilkin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).