examples: print a message in case no pmem provided or able to be used #1185
Conversation
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @Patryk717)
examples/03-read-to-persistent/client.c, line 131 at r1 (raw file):
/* if no pmem support or it is not provided */ if (mr_ptr == NULL) {
No need for adding this empty line.
examples/03-read-to-persistent/client.c, line 132 at r1 (raw file):
if (mr_ptr == NULL) { printer(); /*add printf */
Rename this method to sth more informative, ie. print_no_pmem_detected or sth like that.
examples/common/common-conn.h, line 64 at r1 (raw file):
int common_wait_for_conn_close_and_disconnect(struct rpma_conn **conn_ptr); int common_disconnect_and_wait_for_conn_close(struct rpma_conn **conn_ptr); void printer();
Add an extra line below.
examples/common/common-conn.c, line 201 at r1 (raw file):
printer() { printf("No pmem supportt or pow to pmem no provity \n"); /*add printf */
Perhaps "PMEM not detected or a path to PMEM not provided. Example usage of functions using PMEM skipped.", or sth similar.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @Patryk717)
examples/common/common-conn.h, line 2 at r1 (raw file):
/* SPDX-License-Identifier: BSD-3-Clause */ /* Copyright 2020, Intel Corporation */
2020-2021
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @PatKamin and @Patryk717)
a discussion (no related file):
- You have to add a prefix at the beginning of the commit:
FAIL: subject line in commit message does not contain valid area name
commit eb67b0572a4d7b2ee21f57aad5f84210488b1328
Author: Patryk717 <patrykzbierski717@wp.pl>
Date: Sun Aug 1 21:28:27 2021 +0200
Print message in case no pmem detected in examples
Modified files:
examples/03-read-to-persistent/client.c
examples/common/common-conn.c
examples/common/common-conn.h
Areas computed basing on the list of modified files: (see utils/check-area.sh for full algorithm)
examples
Ref: https://github.com/pmem/rpma/pull/1185/checks?check_run_id=3239654329
- Please add to the commit message:
Ref: #1089
So, effectively the commit message will look as follow:
examples: print a message in case no pmem provided or able to be used
Ref: #1089
examples/03-read-to-persistent/client.c, line 132 at r1 (raw file):
if (mr_ptr == NULL) { printer(); /*add printf */
(void) fprintf(stderr, NO_PMEM_MSG);The NO_PMEM_MSG will describe the situation precisely, because of the way it is defined. Please see other comments.
examples/common/common-conn.h, line 20 at r1 (raw file):
#define SIGNATURE_LEN (strlen(SIGNATURE_STR) + 1) #endif
#if
#define NO_PMEM_MSG "No <pmem-path> provided. Using DRAM instead."
/* ... */
#else
#define NO_PMEM_MSG "The example is unable to use libpmem. If unintended please check the build log. Using DRAM instead."
#endifNote:
- When
USE_LIBPMEMis not defined it means the example binary won't even try to use PMEM because this capability was turned off during the build process. So, we can just say the example is unable to use libpmem and refer the end-user to the build log. - When
USE_LIBPMEMis set there are two possibilities:- a) the end-user will provide a
<pmem-path>OR - b) he won't provide a
<pmem-path>
- a) the end-user will provide a
When <pmem-path> is provided (2a) it will be mapped. If the mapping process succeeds we have nothing to do. The PMEM is there. If any error occurs during the mapping process it will be printed where it happens and the process will be terminated. So, nothing to do for us.
The only interesting case is 2b where <pmem-path> is not provided. And only this case is covered in the proposition above.
examples/common/common-conn.c, line 199 at r1 (raw file):
void printer()
FYI Having a function to just call printf() is overkill.
examples/common/common-conn.c, line 201 at r1 (raw file):
Previously, PatKamin (Patryk Kaminski) wrote…
Perhaps "PMEM not detected or a path to PMEM not provided. Example usage of functions using PMEM skipped.", or sth similar.
I don't like the idea of having 'A or B' message. I think we can do better. Please see other comments.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r2.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @PatKamin and @Patryk717)
a discussion (no related file):
Please squash all commits to a single one.
examples/common/common-conn.h, line 2 at r2 (raw file):
/* SPDX-License-Identifier: BSD-3-Clause */ /* Copyright 2020-2021 , Intel Corporation */
No tab after 2021. Please remove.
examples/common/common-conn.h, line 66 at r2 (raw file):
int common_wait_for_conn_close_and_disconnect(struct rpma_conn **conn_ptr); int common_disconnect_and_wait_for_conn_close(struct rpma_conn **conn_ptr);
This newline is not needed here.
janekmi
changed the title
There was a problem hiding this comment.
Reviewable status: all files reviewed, 12 unresolved discussions (waiting on @PatKamin and @Patryk717)
a discussion (no related file):
You have a few cstyle issues:
/rpma/examples/common/common-conn.h:21: line > 80 characters
/rpma/examples/03-read-to-persistent/client.c:132: continuation line not indented by 4 spaces
/rpma/examples/03-read-to-persistent/client.c:133: continuation line not indented by 4 spaces
/rpma/examples/03-read-to-persistent/client.c:133: left brace starting a line
Ref: https://github.com/pmem/rpma/pull/1185/checks?check_run_id=3280142304
examples/03-read-to-persistent/client.c, line 130 at r2 (raw file):
/* if no pmem support or it is not provided */ if (mr_ptr == NULL)
What this new space is doing here? ;-)
examples/03-read-to-persistent/client.c, line 131 at r2 (raw file):
/* if no pmem support or it is not provided */ if (mr_ptr == NULL) {
What has happened with this brace? ;-)
examples/common/common-conn.h, line 21 at r2 (raw file):
#define NO_PMEM_MSG "No <pmem-path> provided. Using DRAM instead. \n" #else #define NO_PMEM_MSG "The example is unable to use libpmem. If unintended please check the build log. Using DRAM instead. \n"
You have to break this line as follow:
#define NO_PMEM_MSG \
"The example is unable to use libpmem. If unintended please check the build log. Using DRAM instead.\n"Note: Spaces and indentations are important. ;-)
Patryk717
force-pushed
the
printf_test
branch
4 times, most recently
from
e16df61
to
29e2859
Compare
There was a problem hiding this comment.
Reviewable status: 1 of 3 files reviewed, 12 unresolved discussions (waiting on @janekmi, @osalyk, @PatKamin, and @Patryk717)
a discussion (no related file):
Previously, janekmi (Jan Michalski) wrote…
Please squash all commits to a single one.
Done.
examples/03-read-to-persistent/client.c, line 131 at r1 (raw file):
Previously, PatKamin (Patryk Kaminski) wrote…
No need for adding this empty line.
Done.
examples/03-read-to-persistent/client.c, line 132 at r1 (raw file):
Previously, PatKamin (Patryk Kaminski) wrote…
Rename this method to sth more informative, ie. print_no_pmem_detected or sth like that.
Done.
examples/03-read-to-persistent/client.c, line 130 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
What this new space is doing here? ;-)
Done.
examples/03-read-to-persistent/client.c, line 131 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
What has happened with this brace? ;-)
Done.
examples/common/common-conn.h, line 2 at r1 (raw file):
Previously, osalyk (Oksana Sałyk) wrote…
2020-2021
Done.
examples/common/common-conn.h, line 64 at r1 (raw file):
Previously, PatKamin (Patryk Kaminski) wrote…
Add an extra line below.
Done.
examples/common/common-conn.h, line 2 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
No tab after
2021. Please remove.
Done.
examples/common/common-conn.h, line 66 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
This newline is not needed here.
Done.
examples/common/common-conn.c, line 201 at r1 (raw file):
Previously, janekmi (Jan Michalski) wrote…
I don't like the idea of having 'A or B' message. I think we can do better. Please see other comments.
Done.
There was a problem hiding this comment.
Reviewed 1 of 2 files at r3, 1 of 1 files at r5, 1 of 1 files at r6.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @janekmi, @osalyk, and @Patryk717)
There was a problem hiding this comment.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @janekmi and @osalyk)
a discussion (no related file):
Previously, janekmi (Jan Michalski) wrote…
You have a few cstyle issues:
/rpma/examples/common/common-conn.h:21: line > 80 characters /rpma/examples/03-read-to-persistent/client.c:132: continuation line not indented by 4 spaces /rpma/examples/03-read-to-persistent/client.c:133: continuation line not indented by 4 spaces /rpma/examples/03-read-to-persistent/client.c:133: left brace starting a lineRef: https://github.com/pmem/rpma/pull/1185/checks?check_run_id=3280142304
Done.
examples/common/common-conn.h, line 21 at r2 (raw file):
Previously, janekmi (Jan Michalski) wrote…
You have to break this line as follow:
#define NO_PMEM_MSG \ "The example is unable to use libpmem. If unintended please check the build log. Using DRAM instead.\n"Note: Spaces and indentations are important. ;-)
Done.
There was a problem hiding this comment.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @janekmi and @osalyk)
There was a problem hiding this comment.
Reviewed 1 of 2 files at r3, 1 of 1 files at r6.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @osalyk)
Ref: #1089
This change is