| Message ID | 20211104062032.1505899-1-yang.guang5@zte.com.cn (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | tools: iio: use swap() to make code cleaner | expand |
On Thu, Nov 4, 2021 at 8:21 AM <davidcomponentone@gmail.com> wrote: > > From: Yang Guang <yang.guang5@zte.com.cn> > > Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid > opencoding it. Same as per other patches. Don't be a dump addon to the robot, think about the code a little bit more. > void bsort_channel_array_by_index(struct iio_channel_info *ci_array, int cnt) > { > - struct iio_channel_info temp; > int x, y; > > for (x = 0; x < cnt; x++) > for (y = 0; y < (cnt - 1); y++) > if (ci_array[y].index > ci_array[y + 1].index) { > - temp = ci_array[y + 1]; > - ci_array[y + 1] = ci_array[y]; > - ci_array[y] = temp; > + swap(ci_array[y + 1], ci_array[y]); > } Name of the function suggests it's a sort, we have the sort_r() API, use it instead. > }
On Thu, Nov 4, 2021 at 12:33 PM Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, Nov 4, 2021 at 8:21 AM <davidcomponentone@gmail.com> wrote: > > Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid > > opencoding it. > > Same as per other patches. Don't be a dump addon to the robot, think > about the code a little bit more. > > > void bsort_channel_array_by_index(struct iio_channel_info *ci_array, int cnt) > > { > > - struct iio_channel_info temp; > > int x, y; > > > > for (x = 0; x < cnt; x++) > > for (y = 0; y < (cnt - 1); y++) > > if (ci_array[y].index > ci_array[y + 1].index) { > > - temp = ci_array[y + 1]; > > - ci_array[y + 1] = ci_array[y]; > > - ci_array[y] = temp; > > + swap(ci_array[y + 1], ci_array[y]); > > } > > Name of the function suggests it's a sort, we have the sort_r() API, > use it instead. Ah, it's the tools folder... Anyway, the first part of the comment stays valid for all your contribution.
On Thu, 4 Nov 2021 12:36:52 +0200 Andy Shevchenko <andy.shevchenko@gmail.com> wrote: > On Thu, Nov 4, 2021 at 12:33 PM Andy Shevchenko > <andy.shevchenko@gmail.com> wrote: > > On Thu, Nov 4, 2021 at 8:21 AM <davidcomponentone@gmail.com> wrote: > > > > Use the macro 'swap()' defined in 'include/linux/minmax.h' to avoid > > > opencoding it. > > > > Same as per other patches. Don't be a dump addon to the robot, think > > about the code a little bit more. > > > > > void bsort_channel_array_by_index(struct iio_channel_info *ci_array, int cnt) > > > { > > > - struct iio_channel_info temp; > > > int x, y; > > > > > > for (x = 0; x < cnt; x++) > > > for (y = 0; y < (cnt - 1); y++) > > > if (ci_array[y].index > ci_array[y + 1].index) { > > > - temp = ci_array[y + 1]; > > > - ci_array[y + 1] = ci_array[y]; > > > - ci_array[y] = temp; > > > + swap(ci_array[y + 1], ci_array[y]); > > > } > > > > Name of the function suggests it's a sort, we have the sort_r() API, > > use it instead. > > Ah, it's the tools folder... > Anyway, the first part of the comment stays valid for all your contribution. > Also check this builds as it doesn't for me precisely because this is in tools and hence should be using uapi includes only. Maybe there is one going in for this during the merge window but it's not in my tree yet at least. Jonathan
diff --git a/tools/iio/iio_utils.c b/tools/iio/iio_utils.c index aadee6d34c74..7a56c41fbded 100644 --- a/tools/iio/iio_utils.c +++ b/tools/iio/iio_utils.c @@ -290,15 +290,12 @@ int iioutils_get_param_float(float *output, const char *param_name, void bsort_channel_array_by_index(struct iio_channel_info *ci_array, int cnt) { - struct iio_channel_info temp; int x, y; for (x = 0; x < cnt; x++) for (y = 0; y < (cnt - 1); y++) if (ci_array[y].index > ci_array[y + 1].index) { - temp = ci_array[y + 1]; - ci_array[y + 1] = ci_array[y]; - ci_array[y] = temp; + swap(ci_array[y + 1], ci_array[y]); } }