[PATCH] remove unnecessary leading '/' from filename

[PATCH] remove unnecessary leading '/' from filename

[Stan C. Smith]

In osm_db_domain_init() the filename is appended to a path string 'db_dir_name' which contains a trailing '/'.
Remove extra '/'.

Signed-off-by: stan smith <stan.smith@intel.com>

diff --git a/opensm/opensm/osm_lid_mgr.c b/opensm/opensm/osm_lid_mgr.c
index 947fe7a..1704054 100644
--- a/opensm/opensm/osm_lid_mgr.c
+++ b/opensm/opensm/osm_lid_mgr.c
@@ -239,7 +239,7 @@ ib_api_status_t osm_lid_mgr_init(IN osm_lid_mgr_t * p_mgr, IN osm_sm_t * sm)
 	p_mgr->p_lock = sm->p_lock;
 
 	/* we initialize and restore the db domain of guid to lid map */
-	p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "/guid2lid");
+	p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "guid2lid");
 	if (!p_mgr->p_g2l) {
 		OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 0316: "
 			"Error initializing Guid-to-Lid persistent database\n");

Re: [PATCH] remove unnecessary leading '/' from filename

[Sasha Khapyorsky]

On 16:46 Wed 07 Oct     , Stan C. Smith wrote:
> 
> In osm_db_domain_init() the filename is appended to a path string 'db_dir_name' which contains a trailing '/'.
> Remove extra '/'.
> 
> Signed-off-by: stan smith <stan.smith@intel.com>
> 
> diff --git a/opensm/opensm/osm_lid_mgr.c b/opensm/opensm/osm_lid_mgr.c
> index 947fe7a..1704054 100644
> --- a/opensm/opensm/osm_lid_mgr.c
> +++ b/opensm/opensm/osm_lid_mgr.c
> @@ -239,7 +239,7 @@ ib_api_status_t osm_lid_mgr_init(IN osm_lid_mgr_t * p_mgr, IN osm_sm_t * sm)
>  	p_mgr->p_lock = sm->p_lock;
>  
>  	/* we initialize and restore the db domain of guid to lid map */
> -	p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "/guid2lid");
> +	p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "guid2lid");

I think that this can break when OSM_CACHE_DIR is supplied as
environment variable and its value doesn't have '/' at the end. The
patch below is from 2007 and it fixed this issue.

Sasha


commit 907b95ec02d6a794ee8f6d6996cc6350a476bd15
Author: Albert L. Chu <chu11@llnl.gov>
Date:   Wed Mar 14 07:08:58 2007 -0500

    OpenSM: Fix cache filename corner case
    
    Signed-off-by: Albert L. Chu <chu11@llnl.gov>
    Signed-off-by: Hal Rosenstock <halr@voltaire.com>

diff --git a/osm/opensm/osm_lid_mgr.c b/osm/opensm/osm_lid_mgr.c
index 89adc48..bd5945f 100644
--- a/osm/opensm/osm_lid_mgr.c
+++ b/osm/opensm/osm_lid_mgr.c
@@ -282,7 +282,7 @@ osm_lid_mgr_init(
   p_mgr->p_req = p_req;
 
   /* we initialize and restore the db domain of guid to lid map */
-  p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "guid2lid");
+  p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "/guid2lid");
   if (! p_mgr->p_g2l)
   {
     osm_log( p_mgr->p_log, OSM_LOG_ERROR,
diff --git a/osm/opensm/osm_subnet.c b/osm/opensm/osm_subnet.c
index 99e39aa..cbb3549 100644
--- a/osm/opensm/osm_subnet.c
+++ b/osm/opensm/osm_subnet.c
@@ -745,7 +745,7 @@ osm_subn_rescan_conf_file(
     p_cache_dir = OSM_DEFAULT_CACHE_DIR;
 
   strcpy(file_name, p_cache_dir);
-  strcat(file_name, "opensm.opts");
+  strcat(file_name, "/opensm.opts");
   
   opts_file = fopen(file_name, "r");
   if (!opts_file)
@@ -838,7 +838,7 @@ osm_subn_parse_conf_file(
     p_cache_dir = OSM_DEFAULT_CACHE_DIR;
 
   strcpy(file_name, p_cache_dir);
-  strcat(file_name, "opensm.opts");
+  strcat(file_name, "/opensm.opts");
   
   opts_file = fopen(file_name, "r");
   if (!opts_file) return;
@@ -1096,7 +1096,7 @@ osm_subn_write_conf_file(
     p_cache_dir = OSM_DEFAULT_CACHE_DIR;
 
   strcpy(file_name, p_cache_dir);
-  strcat(file_name, "opensm.opts");
+  strcat(file_name, "/opensm.opts");
   
   opts_file = fopen(file_name, "w");
   if (!opts_file) return;

>  	if (!p_mgr->p_g2l) {
>  		OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 0316: "
>  			"Error initializing Guid-to-Lid persistent database\n");
> 

RE: [PATCH] remove unnecessary leading '/' from filename

[Smith, Stan]

Sasha Khapyorsky wrote:
> On 16:46 Wed 07 Oct     , Stan C. Smith wrote:
>>
>> In osm_db_domain_init() the filename is appended to a path string
>> 'db_dir_name' which contains a trailing '/'. Remove extra '/'.
>>
>> Signed-off-by: stan smith <stan.smith@intel.com>
>>
>> diff --git a/opensm/opensm/osm_lid_mgr.c
>> b/opensm/opensm/osm_lid_mgr.c
>> index 947fe7a..1704054 100644
>> --- a/opensm/opensm/osm_lid_mgr.c
>> +++ b/opensm/opensm/osm_lid_mgr.c
>> @@ -239,7 +239,7 @@ ib_api_status_t osm_lid_mgr_init(IN
>> osm_lid_mgr_t * p_mgr, IN osm_sm_t * sm)     p_mgr->p_lock =
>> sm->p_lock;
>>
>>      /* we initialize and restore the db domain of guid to lid map */
>> -    p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "/guid2lid");
>> +    p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "guid2lid");
>
> I think that this can break when OSM_CACHE_DIR is supplied as
> environment variable and its value doesn't have '/' at the end. The
> patch below is from 2007 and it fixed this issue.
>
> Sasha

I believe removing the starting '/' from guid2lid filename is the correct fix for a couple of reasons:
1) prefixing a / to a filename seems strange when the path name is spec'ed as having a trailing / ?
2) / in a windows filename is a problem.

Due to comments in opensm/include/opensm/osm_base.h which explicitly speak to the requirement for a trailing directory delimiter, the missing functionality is the check for the trailing directory delimiter when using an environment var to build a path. One could be a 'good' guy and if the env is missing the trailing / then add one.

Do want me to resubmit with the env var check/fix or do you want to do it?

Stan.

>
>
> commit 907b95ec02d6a794ee8f6d6996cc6350a476bd15
> Author: Albert L. Chu <chu11@llnl.gov>
> Date:   Wed Mar 14 07:08:58 2007 -0500
>
>     OpenSM: Fix cache filename corner case
>
>     Signed-off-by: Albert L. Chu <chu11@llnl.gov>
>     Signed-off-by: Hal Rosenstock <halr@voltaire.com>
>
> diff --git a/osm/opensm/osm_lid_mgr.c b/osm/opensm/osm_lid_mgr.c
> index 89adc48..bd5945f 100644
> --- a/osm/opensm/osm_lid_mgr.c
> +++ b/osm/opensm/osm_lid_mgr.c
> @@ -282,7 +282,7 @@ osm_lid_mgr_init(
>    p_mgr->p_req = p_req;
>
>    /* we initialize and restore the db domain of guid to lid map */
> -  p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "guid2lid");
> +  p_mgr->p_g2l = osm_db_domain_init(p_mgr->p_db, "/guid2lid");
>    if (! p_mgr->p_g2l)
>    {
>      osm_log( p_mgr->p_log, OSM_LOG_ERROR,
> diff --git a/osm/opensm/osm_subnet.c b/osm/opensm/osm_subnet.c
> index 99e39aa..cbb3549 100644
> --- a/osm/opensm/osm_subnet.c
> +++ b/osm/opensm/osm_subnet.c
> @@ -745,7 +745,7 @@ osm_subn_rescan_conf_file(
>      p_cache_dir = OSM_DEFAULT_CACHE_DIR;
>
>    strcpy(file_name, p_cache_dir);
> -  strcat(file_name, "opensm.opts");
> +  strcat(file_name, "/opensm.opts");
>
>    opts_file = fopen(file_name, "r");
>    if (!opts_file)
> @@ -838,7 +838,7 @@ osm_subn_parse_conf_file(
>      p_cache_dir = OSM_DEFAULT_CACHE_DIR;
>
>    strcpy(file_name, p_cache_dir);
> -  strcat(file_name, "opensm.opts");
> +  strcat(file_name, "/opensm.opts");
>
>    opts_file = fopen(file_name, "r");
>    if (!opts_file) return;
> @@ -1096,7 +1096,7 @@ osm_subn_write_conf_file(
>      p_cache_dir = OSM_DEFAULT_CACHE_DIR;
>
>    strcpy(file_name, p_cache_dir);
> -  strcat(file_name, "opensm.opts");
> +  strcat(file_name, "/opensm.opts");
>
>    opts_file = fopen(file_name, "w");
>    if (!opts_file) return;
>
>>      if (!p_mgr->p_g2l) {
>>              OSM_LOG(p_mgr->p_log, OSM_LOG_ERROR, "ERR 0316: "
>>                      "Error initializing Guid-to-Lid persistent database\n");

Re: [PATCH] remove unnecessary leading '/' from filename

[Sasha Khapyorsky]

Hi Stan,

On 15:18 Mon 12 Oct     , Smith, Stan wrote:
> 
> I believe removing the starting '/' from guid2lid filename is the correct fix for a couple of reasons:
> 1) prefixing a / to a filename seems strange when the path name is spec'ed as having a trailing / ?

I agree. We need to care about making path properly rather than
introduce a restrictions for directory names definitions.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] opensm/osm_db_files.c: add '/' path delimited

[Sasha Khapyorsky]

Add '/' path delimiter unconditionally on file path generation.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/opensm/osm_db_files.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/opensm/opensm/osm_db_files.c b/opensm/opensm/osm_db_files.c
index ec7436e..5fd7c83 100644
--- a/opensm/opensm/osm_db_files.c
+++ b/opensm/opensm/osm_db_files.c
@@ -195,7 +195,7 @@ osm_db_domain_t *osm_db_domain_init(IN osm_db_t * p_db, IN char *domain_name)
 {
 	osm_db_domain_t *p_domain;
 	osm_db_domain_imp_t *p_domain_imp;
-	int dir_name_len;
+	size_t path_len;
 	osm_log_t *p_log = p_db->p_log;
 	FILE *p_file;
 
@@ -209,16 +209,14 @@ osm_db_domain_t *osm_db_domain_init(IN osm_db_t * p_db, IN char *domain_name)
 	    (osm_db_domain_imp_t *) malloc(sizeof(osm_db_domain_imp_t));
 	CL_ASSERT(p_domain_imp != NULL);
 
-	dir_name_len = strlen(((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name);
+	path_len = strlen(((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name)
+	    + strlen(domain_name) + 2;
 
 	/* set the domain file name */
-	p_domain_imp->file_name =
-	    (char *)malloc(sizeof(char) * (dir_name_len) + strlen(domain_name) +
-			   2);
+	p_domain_imp->file_name = malloc(path_len);
 	CL_ASSERT(p_domain_imp->file_name != NULL);
-	strcpy(p_domain_imp->file_name,
-	       ((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name);
-	strcat(p_domain_imp->file_name, domain_name);
+	snprintf(p_domain_imp->file_name, path_len, "%s/%s",
+		 ((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name, domain_name);
 
 	/* make sure the file exists - or exit if not writable */
 	p_file = fopen(p_domain_imp->file_name, "a+");
-- 
1.6.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH] opensm/osm_base.h: remove trailing '/' in OSM_DEFAULT_CACHE_DIR

[Sasha Khapyorsky]

Remove trailing '/' character in OSM_DEFAULT_CACHE_DIR definition as
well as misleading comment about its mandatory status.

Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
---
 opensm/include/opensm/osm_base.h |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)

diff --git a/opensm/include/opensm/osm_base.h b/opensm/include/opensm/osm_base.h
index 9d8bf98..4e9aaa9 100644
--- a/opensm/include/opensm/osm_base.h
+++ b/opensm/include/opensm/osm_base.h
@@ -192,14 +192,13 @@ BEGIN_C_DECLS
 *
 * DESCRIPTION
 *	Specifies the default cache directory for the db files.
-*	Note that the directory must appear with "/" ("\\" for windows) at the end.
 *
 * SYNOPSIS
 */
 #ifdef __WIN__
 #define OSM_DEFAULT_CACHE_DIR GetOsmCachePath()
 #else
-#define OSM_DEFAULT_CACHE_DIR "/var/cache/opensm/"
+#define OSM_DEFAULT_CACHE_DIR "/var/cache/opensm"
 #endif
 /***********/
 /****d* OpenSM: Base/OSM_DEFAULT_LOG_FILE
-- 
1.6.5.3

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remove unnecessary leading '/' from filename

[Sasha Khapyorsky]

On 16:46 Wed 07 Oct     , Stan C. Smith wrote:
> 
> In osm_db_domain_init() the filename is appended to a path string 'db_dir_name' which contains a trailing '/'.
> Remove extra '/'.
> 
> Signed-off-by: stan smith <stan.smith-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Applied. Thanks.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] remove unnecessary leading '/' from filename

[Smith, Stan]

Sasha Khapyorsky wrote:
> Hi Stan,
>
> On 15:18 Mon 12 Oct     , Smith, Stan wrote:
>>
>> I believe removing the starting '/' from guid2lid filename is the
>> correct fix for a couple of reasons: 1) prefixing a / to a filename
>> seems strange when the path name is spec'ed as having a trailing / ?
>
> I agree. We need to care about making path properly rather than
> introduce a restrictions for directory names definitions.
>
> Sasha

Hello,
  OK on proper paths.
What about the excellent point you raised in previous discussions about those pathnames which are input from environment vars which could be improperly formed? For badly formed paths from ENV vars (lacking trailing '/'), is it sufficient to let open() fail and output path/filename to osm log file?

Stan.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] opensm/osm_db_files.c: add '/' path delimited

[Smith, Stan]

Sasha Khapyorsky wrote:
> Add '/' path delimiter unconditionally on file path generation.

Any chance you could make the path delimiter a #define so there is not an explicit Linux filesystem assumption?


>
> Signed-off-by: Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org>
> ---
>  opensm/opensm/osm_db_files.c |   14 ++++++--------
>  1 files changed, 6 insertions(+), 8 deletions(-)
>
> diff --git a/opensm/opensm/osm_db_files.c
> b/opensm/opensm/osm_db_files.c
> index ec7436e..5fd7c83 100644
> --- a/opensm/opensm/osm_db_files.c
> +++ b/opensm/opensm/osm_db_files.c
> @@ -195,7 +195,7 @@ osm_db_domain_t *osm_db_domain_init(IN osm_db_t *
>  p_db, IN char *domain_name) {
>       osm_db_domain_t *p_domain;
>       osm_db_domain_imp_t *p_domain_imp;
> -     int dir_name_len;
> +     size_t path_len;
>       osm_log_t *p_log = p_db->p_log;
>       FILE *p_file;
>
> @@ -209,16 +209,14 @@ osm_db_domain_t *osm_db_domain_init(IN osm_db_t
>           * p_db, IN char *domain_name) (osm_db_domain_imp_t *)
>       malloc(sizeof(osm_db_domain_imp_t)); CL_ASSERT(p_domain_imp !=
> NULL);
>
> -     dir_name_len = strlen(((osm_db_imp_t *)
> p_db->p_db_imp)->db_dir_name); +      path_len = strlen(((osm_db_imp_t *)
> p_db->p_db_imp)->db_dir_name) +           + strlen(domain_name) + 2;
>
>       /* set the domain file name */
> -     p_domain_imp->file_name =
> -         (char *)malloc(sizeof(char) * (dir_name_len) +
> strlen(domain_name) +
> -                        2);
> +     p_domain_imp->file_name = malloc(path_len);
>       CL_ASSERT(p_domain_imp->file_name != NULL);
> -     strcpy(p_domain_imp->file_name,
> -            ((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name);
> -     strcat(p_domain_imp->file_name, domain_name);
> +     snprintf(p_domain_imp->file_name, path_len, "%s/%s",
> +              ((osm_db_imp_t *) p_db->p_db_imp)->db_dir_name, domain_name);
>
>       /* make sure the file exists - or exit if not writable */
>       p_file = fopen(p_domain_imp->file_name, "a+");

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] opensm/osm_db_files.c: add '/' path delimited

[Bart Van Assche]

On Sat, Nov 28, 2009 at 5:35 PM, Smith, Stan <stan.smith-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> Sasha Khapyorsky wrote:
> > Add '/' path delimiter unconditionally on file path generation.
>
> Any chance you could make the path delimiter a #define so there is not an explicit Linux filesystem assumption?

As far as I know the Windows kernel recognizes both forward and
backward slashes as a path separator.

Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] opensm/osm_db_files.c: add '/' path delimited

[Smith, Stan]

Bart Van Assche wrote:
> On Sat, Nov 28, 2009 at 5:35 PM, Smith, Stan <stan.smith-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> wrote:
>>
>> Sasha Khapyorsky wrote:
>>> Add '/' path delimiter unconditionally on file path generation.
>>
>> Any chance you could make the path delimiter a #define so there is
>> not an explicit Linux filesystem assumption?
>
> As far as I know the Windows kernel recognizes both forward and
> backward slashes as a path separator.
>
> Bart.

Who would of thought...windows _open() & fopen() can now tolerate a mix-n-match of path delimiters; Implementations do change.

Sasha, skip the #define thought.

Thanks Bart.
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] remove unnecessary leading '/' from filename

[Sasha Khapyorsky]

On 08:30 Sat 28 Nov     , Smith, Stan wrote:
> What about the excellent point you raised in previous discussions about those pathnames which are input from environment vars which could be improperly formed? For badly formed paths from ENV vars (lacking trailing '/'), is it sufficient to let open() fail and output path/filename to osm log file?

We are adding delimiter now (in a file path building code), so it is
not mandatory to have trailing '/' in directory name environment
variable.

Sasha
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html