You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "g2gps (via GitHub)" <gi...@apache.org> on 2023/11/17 04:40:08 UTC
[PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
g2gps opened a new pull request, #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201
## Summary
Add the static keyword to required function and variable delcaration to reduce their scope. Stops namespace pollution in `CONFIG_BUILD_FLAT`
## Impact
Stops symbol collision when `CONFIG_TESTING_SD_BENCH` and `CONFIG_TESTING_SD_STRESS` are enables simultaneously.
## Testing
Enable `CONFIG_TESTING_SD_BENCH` and `CONFIG_TESTING_SD_STRESS` at the same time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201#discussion_r1396772343
##########
testing/sd_stress/sd_stress_main.c:
##########
@@ -272,14 +272,14 @@ static bool rename_dir(const char *old_dir, const char *new_dir)
return true;
}
-struct timespec get_abs_time(void)
+static struct timespec get_abs_time(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
return ts;
}
-uint64_t get_time_delta(const struct timespec *start,
+static uint64_t get_time_delta(const struct timespec *start,
const struct timespec *end)
Review Comment:
```suggestion
const struct timespec *end)
```
##########
testing/sd_bench/sd_bench_main.c:
##########
@@ -107,26 +107,26 @@ static int read_test(int fd, sdb_config_t *cfg, uint8_t *block,
int block_size);
static uint64_t time_fsync_us(int fd);
-struct timespec get_abs_time(void);
-uint64_t get_elapsed_time_us(const struct timespec *start);
-uint64_t time_fsync_us(int fd);
-float ts_to_kb(uint64_t bytes, uint64_t elapsed);
-float block_count_to_mb(size_t blocks, size_t block_size);
+static struct timespec get_abs_time(void);
+static uint64_t get_elapsed_time_us(const struct timespec *start);
+static uint64_t time_fsync_us(int fd);
+static float ts_to_kb(uint64_t bytes, uint64_t elapsed);
+static float block_count_to_mb(size_t blocks, size_t block_size);
static const char *print_bool(const bool value);
static void usage(void);
/****************************************************************************
* Private Functions
****************************************************************************/
-struct timespec get_abs_time(void)
+static struct timespec get_abs_time(void)
{
struct timespec ts;
clock_gettime(CLOCK_MONOTONIC, &ts);
return ts;
}
-uint64_t get_time_delta_us(const struct timespec *start,
+static uint64_t get_time_delta_us(const struct timespec *start,
const struct timespec *end)
Review Comment:
```suggestion
const struct timespec *end)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "davids5 (via GitHub)" <gi...@apache.org>.
davids5 commented on PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201#issuecomment-1819138950
Yokogawa 700937
> > Why not allocate these in a struct and reduce memory footprint that just including that app costs?
>
> Could you please expand more with maybe a kind of example. I'm not fully following your proposal. I think all those local `static const`s will be optimized out by compiler anyway.
I did not think these were local. Also as globals they do not conform to the coding standard.
My point was placing all the variables in a struct allocated at run time will reduce the data size to a single pointer or 0 if it is passed as an argument.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201#issuecomment-2083610660
ping @g2gps
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "g2gps (via GitHub)" <gi...@apache.org>.
g2gps commented on PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201#issuecomment-2099782417
Sorry, I'd forgotten that this was pending.
These variables are marked static, so if I understand correctly, there scope is limited to the files the file they are declared in.
There is an opportunity to reduce the scope of some of these variables, if required I can refactor.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] testing/sd_*: Restrict function and variable scope to current file. [nuttx-apps]
Posted by "pkarashchenko (via GitHub)" <gi...@apache.org>.
pkarashchenko commented on PR #2201:
URL: https://github.com/apache/nuttx-apps/pull/2201#issuecomment-1816462938
> Why not allocate these in a struct and reduce memory footprint that just including that app costs?
Could you please expand more with maybe a kind of example. I'm not fully following your proposal. I think all those local `const`s will be optimized out by compiler anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org