Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Tue, 24 Aug 2021 17:00:06 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     sidraya.bj@pathpartnertech.com
Cc:     gregkh@linuxfoundation.org, linux-staging@lists.linux.dev,
        linux-kernel@vger.kernel.org, prashanth.ka@pathpartnertech.com,
        praneeth@ti.com, mchehab@kernel.org, linux-media@vger.kernel.org,
        praveen.ap@pathpartnertech.com
Subject: Re: [PATCH 09/30] v4l: vxd-dec: Add idgen api modules
Message-ID: <20210824140005.GQ1931@kadam>
References: <20210818141037.19990-1-sidraya.bj@pathpartnertech.com>
 <20210818141037.19990-10-sidraya.bj@pathpartnertech.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20210818141037.19990-10-sidraya.bj@pathpartnertech.com>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-MessageSentRepresentingType: 1
Received: from kadam (62.8.83.99) by JNAP275CA0018.ZAFP275.PROD.OUTLOOK.COM (2603:1086:0:4c::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4457.17 via Frontend Transport; Tue, 24 Aug 2021 14:00:18 +0000
X-MS-PublicTrafficType: Email
X-MS-Office365-Filtering-Correlation-Id: 4c3c74c6-1236-4646-0762-08d9670783ed
X-MS-TrafficTypeDiagnostic: CY4PR1001MB2263:
X-MS-Exchange-Transport-Forked: True
X-Microsoft-Antispam-PRVS: <CY4PR1001MB2263C82B23744A8064EDEF5C8EC59@CY4PR1001MB2263.namprd10.prod.outlook.com>
X-MS-Oob-TLC-OOBClassifiers: OLM:2582;
X-MS-Exchange-SenderADCheck: 1
X-MS-Exchange-AntiSpam-Relay: 0
X-Microsoft-Antispam: BCL:0;
X-Microsoft-Antispam-Message-Info: L1fU1GEFiqEJGvIz8JYIcmrwGIexkqeif+BXdOu52Q3FYm9ZpnYt8h/l8GL1LHmJDYj3aqiImycOFIPtDP26/QkqNojawmvSu6pkejBoPjuD/0/9Iq7Vfb6PIgbHQdVg5P1c2XNftz26HwnnWckhbg+9FtoBuUmqBiSeZKsPRZg6RGXxqm0K2YoG93KzqVm8zPwhsXrN/1MxSTliTAl8DAiaCsN0wRe0nzRqW1TwqXe3WkiAT9t6qJD6U9F34rKLhfyu+b3FRXaxAa74mi67rclsQyrVkk1Enbk1k1e636mwhVoOJFuxNfttWOic4c+V1e3yvs3uxpw9O2nXKARHQb+EQ7VP/g2iHPcnjjLDr9eiRM+my6JoRksGGKhxF/7sAKATxXA+rMIT/pZDvFjIpxKXgMVhgRbGbeQiarP8LZEljlrRwuxXDyvZLkfVZdsnV8xmZEtUwoMI4lmVh73hai42QAmU0Ii52ucaK6ctN0YOg2y7Z3P/TVAnJdjfNLUIB25Qp54A8yyLyEe71MIYImtYuqi+3/rEpKg+yaO1HtLblh/OTbDA3Mj6wvXdR230OH9/k8DvnDEv3dUGlQs6uvvcDjsO8W9KQl2uQCy2NqVxotL7r7V23dBRmJgiO6d1wkkEuRImeLlsesFvKh6fbg55sd9MGLQb12zvSMFuXabbAapVA43uOwGT7ogrnDn6M69FSR29KAbQUgl8QFP8zQ==
X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:CY4PR1001MB2358.namprd10.prod.outlook.com;PTR:;CAT:NONE;SFS:(39860400002)(136003)(396003)(366004)(376002)(346002)(8936002)(86362001)(26005)(33716001)(66556008)(44832011)(9576002)(956004)(55016002)(6496006)(9686003)(66476007)(186003)(316002)(478600001)(2906002)(4326008)(83380400001)(5660300002)(6666004)(33656002)(66946007)(38350700002)(38100700002)(6916009)(1076003)(8676002)(52116002);DIR:OUT;SFP:1101;
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?TSavSd7KeFQN3FFOyDWhad7YzHq0dYu4GRZNeSa+g96neX29H50Z0biZf0vg?=
 =?us-ascii?Q?F2fnv2d0GJaMrivVUXbrkdP5bsxckfmN55eQMIs3nM+9uKus7V8l6YDdbdKX?=
 =?us-ascii?Q?iG5mnkXFh8586mG4JOtFP/w7Yfk6cjsoSih2QXh3iEbslMtdvGYII5FJIoJy?=
 =?us-ascii?Q?bfbGkKz5lsZBHLXNkbVoQEhBewoY6I0QzGdZOEg90ZmjI4Ffidq4AOq+zYxc?=
 =?us-ascii?Q?wsdZlS/dwR70DhqLQdwMAdLuC/RvT3VVwOmEDQGjjWafMb64eyy7JHQ727QI?=
 =?us-ascii?Q?TzU4pvXU1bRJJvjyD55y03PclFMVHn0MNATFn69OsWEMS0sdH+OGDpOeRwRa?=
 =?us-ascii?Q?6fDeTw/vMMlk+ePizUbJABDUwS+6FygPaikUswBvuslpcmQdv7HRIBr2YVeG?=
 =?us-ascii?Q?E/noGOjpWP16jq7osjFLcYFqzKZ+TNpfA7RMKJgANRpPu5uaILj+pJYcETZY?=
 =?us-ascii?Q?f3yGu+DZcngd4hVEKj3UT15iUKazaPhi7EpmzOSp0afOBN2K7OlumkJMBl4l?=
 =?us-ascii?Q?aeOSgiAFgz0PG97o8J1SAgB54j33wewaHRNGJ9ELszlZB3x6AsazivH6K4Ia?=
 =?us-ascii?Q?odoYaDJgk9mM0MPUcH1morzSbh6BCwCe2oyy3QNK4yXFNHs+SRSK+8KbZ5Ge?=
 =?us-ascii?Q?hkPgCpmVtmOj448vHtEcBjenpduxls4DIVt+eI4KPG4wGxwK4DRQmqJPbbG7?=
 =?us-ascii?Q?/KpZAUcrzZqlre/yppxEqhv9+CxHGZQTovRSWoPoE97y/LgfoUxhdtosJQMx?=
 =?us-ascii?Q?+sQnxp0AfuaYgfIe0gmXn4loXNVK0NvA/12A2L9gcRVXiG9TIgsPycFb5voY?=
 =?us-ascii?Q?W9dHllDHEmcYOecnslYCPuEuWmA7HUTfEA/V+ORVanUWuCBQ9Ex90ZQOzadv?=
 =?us-ascii?Q?eQD/hKvK5fpv5UyGE7LwPKa/ZUkA/dKMR9xjLO4/lPjT1cCejHD4OYJZanfd?=
 =?us-ascii?Q?SLwpHas9uwzbqkiK+tLyiSubqsE3PhmkUXp62vHGGzNipZULxGGA3kJCejNu?=
 =?us-ascii?Q?KM+4lp/0yu9Is3U5fNhQlpHiXqeCKLceMbVWTYvcd9djvNANbdNgj/8evP9M?=
 =?us-ascii?Q?umNuz6yNXtIPGY2sKm2d95/cDmFdxFT+EAsgebTMDdnjf1H+El9LS6MC8LcC?=
 =?us-ascii?Q?Zu7PrBXJVr2wOpFOJ3YlU9ZhPVZmQOGNj1NNxgis/82BmahAVpZaNopRUV8N?=
 =?us-ascii?Q?P6oXvh66G8LcQjuzQ6jlNhU764y7BHhumTi77Dt7oilwRy/JRuAO6oTgzUhm?=
 =?us-ascii?Q?GJQS14o7c9hs/+vI7jHj83gkGE0zcEWFh8+jnrx2twEGhKBup9VcoBOsXrH8?=
 =?us-ascii?Q?+1jBrDN5YQeQEPp7paLsqYkI?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 4c3c74c6-1236-4646-0762-08d9670783ed
X-MS-Exchange-CrossTenant-AuthSource: CY4PR1001MB2358.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 24 Aug 2021 14:00:23.0893
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: gkTAz+0/owPmXU5ucswu4sNbCiX4rlIUl+TiSjrLkVOqg7KEsBTiQp1zCsuoyhsnzG6PnBCGARy7nW0BiQK6o0xdXN7lXoIsX6QZ4ivvmDU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY4PR1001MB2263
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10085 signatures=668682
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 spamscore=0 phishscore=0 malwarescore=0
 mlxscore=0 bulkscore=0 mlxlogscore=999 suspectscore=0 adultscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2107140000
 definitions=main-2108240093
X-Proofpoint-ORIG-GUID: dF4sKUg-yIFt2VvSh-X0wlYW5QLynYRJ
X-Proofpoint-GUID: dF4sKUg-yIFt2VvSh-X0wlYW5QLynYRJ
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Of course this is all staging code and we normally don't review staging
code too hard when it's first sent because we understand that staging
code needs a lot of work before it's acceptable.

One of the rules of staging is that you can't touch code outside of
staging.

But since I happened to glance at a some of this code then here are
some tiny comments:

On Wed, Aug 18, 2021 at 07:40:16PM +0530, sidraya.bj@pathpartnertech.com wrote:
> +/*
> + * Structure contains the ID context.
> + */
> +struct idgen_hdblk {
> +	void **link; /* to be part of single linked list */
> +	/* Array of handles in this block. */
> +	void *ahhandles[1];

Don't use 1 element flex arrays.  Do it like this:

	void *ahhandles[];

You will have to adjust all you math.  But you should be using the
"struct_size(hdblk, ahhandles, nr)" macro anyway to prevent integer
overflows.

> +int idgen_createcontext(unsigned int maxid, unsigned int blksize,
> +			int incid, void **idgenhandle)
> +{
> +	struct idgen_context *idcontext;
> +
> +	/* Create context structure */
> +	idcontext = kzalloc(sizeof(*idcontext), GFP_KERNEL);
> +	if (!idcontext)
> +		return IMG_ERROR_OUT_OF_MEMORY;

We need to get rid of all these weird error codes?  You can add that
to the TODO file.

> +int idgen_destroycontext(void *idgenhandle)
> +{
> +	struct idgen_context *idcontext = (struct idgen_context *)idgenhandle;
> +	struct idgen_hdblk *hdblk;
> +
> +	if (!idcontext)
> +		return IMG_ERROR_INVALID_PARAMETERS;
> +
> +	/* If incrementing Ids, free the List of Incrementing Ids */
> +	if (idcontext->incids) {
> +		struct idgen_id *id;
> +		unsigned int i = 0;
> +
> +		for (i = 0; i < idcontext->blksize; i++) {
> +			id = lst_removehead(&idcontext->incidlist[i]);

You're not allowed to invent your own list API. :P  Generally there just
seems like too much extra helper functions and wrappers.  Removing this
kind of stuff is standard for staging drivers so that's normall.  Add it
to the TODO.

regards,
dan carpenter