[PATCH 1/2] DebugDump.cpp: lowercase and rename functions

[Nikola Pajkovsky]

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
---
 lib/utils/DebugDump.cpp |   43 ++++++++++++++++++++++---------------------
 1 files changed, 22 insertions(+), 21 deletions(-)

diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
index 8987b43..37e21c7 100644
--- a/lib/utils/DebugDump.cpp
+++ b/lib/utils/DebugDump.cpp
@@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
     return true;
 }
 
-static char* RemoveBackSlashes(const char *pDir)
+// doesn't it only slash?
+static char* rm_backslashes(const char *pDir)
 {
     unsigned len = strlen(pDir);
     while (len != 0 && pDir[len-1] == '/')
@@ -42,7 +43,7 @@ static char* RemoveBackSlashes(const char *pDir)
     return xstrndup(pDir, len);
 }
 
-static bool ExistFileDir(const char *pPath)
+static bool exist_file_dir(const char *pPath)
 {
     struct stat buf;
     if (stat(pPath, &buf) == 0)
@@ -55,7 +56,7 @@ static bool ExistFileDir(const char *pPath)
     return false;
 }
 
-static char *LoadTextFile(const char *path);
+static char *loadtxt_file(const char *path);
 static void dd_lock(dump_dir_t *dd);
 static void dd_unlock(dump_dir_t *dd);
 
@@ -85,8 +86,8 @@ int dd_opendir(dump_dir_t *dd, const char *dir)
     if (dd->opened)
         error_msg_and_die("CDebugDump is already opened");
 
-    dd->dd_dir = RemoveBackSlashes(dir);
-    if (!dd->dd_dir || !ExistFileDir(dd->dd_dir))
+    dd->dd_dir = rm_backslashes(dir);
+    if (!dd->dd_dir || !exist_file_dir(dd->dd_dir))
     {
         error_msg("'%s' does not exist", dd->dd_dir);
         return 0;
@@ -109,12 +110,12 @@ int dd_opendir(dump_dir_t *dd, const char *dir)
 int dd_exist(dump_dir_t *dd, const char *path)
 {
     char *full_path = concat_path_file(dd->dd_dir, path);
-    int ret = ExistFileDir(full_path);
+    int ret = exist_file_dir(full_path);
     free(full_path);
     return ret;
 }
 
-static bool GetAndSetLock(const char* pLockFile, const char* pPID)
+static bool get_and_set_lock(const char* pLockFile, const char* pPID)
 {
     while (symlink(pPID, pLockFile) != 0)
     {
@@ -232,7 +233,7 @@ static void dd_lock(dump_dir_t *dd)
 
     char pid_buf[sizeof(long)*3 + 2];
     sprintf(pid_buf, "%lu", (long)getpid());
-    while ((dd->locked = GetAndSetLock(lock_buf, pid_buf)) != true)
+    while ((dd->locked = get_and_set_lock(lock_buf, pid_buf)) != true)
     {
         sleep(1); /* was 0.5 seconds */
     }
@@ -273,8 +274,8 @@ int dd_create(dump_dir_t *dd, const char *dir, uid_t uid)
     if (dd->opened)
         error_msg_and_die("DebugDump is already opened");
 
-    dd->dd_dir = RemoveBackSlashes(dir);
-    if (ExistFileDir(dd->dd_dir))
+    dd->dd_dir = rm_backslashes(dir);
+    if (exist_file_dir(dd->dd_dir))
     {
         error_msg("'%s' already exists", dd->dd_dir);
         return 0;
@@ -338,7 +339,7 @@ int dd_create(dump_dir_t *dd, const char *dir, uid_t uid)
         }
         dd_savetxt(dd, FILENAME_KERNEL, buf.release);
         dd_savetxt(dd, FILENAME_ARCHITECTURE, buf.machine);
-        char *release = LoadTextFile("/etc/redhat-release");
+        char *release = loadtxt_file("/etc/redhat-release");
         strchrnul(release, '\n')[0] = '\0';
         dd_savetxt(dd, FILENAME_RELEASE, release);
         free(release);
@@ -352,9 +353,9 @@ int dd_create(dump_dir_t *dd, const char *dir, uid_t uid)
     return 1;
 }
 
-static bool DeleteFileDir(const char *pDir)
+static bool delete_file_dir(const char *pDir)
 {
-    if (!ExistFileDir(pDir))
+    if (!exist_file_dir(pDir))
         return true;
 
     DIR *d = opendir(pDir);
@@ -379,7 +380,7 @@ static bool DeleteFileDir(const char *pDir)
                 free(full_path);
                 return false;
             }
-            DeleteFileDir(full_path);
+            delete_file_dir(full_path);
         }
         free(full_path);
     }
@@ -395,15 +396,15 @@ static bool DeleteFileDir(const char *pDir)
 
 void dd_delete(dump_dir_t *dd)
 {
-    if (!ExistFileDir(dd->dd_dir))
+    if (!exist_file_dir(dd->dd_dir))
     {
         return;
     }
 
-    DeleteFileDir(dd->dd_dir);
+    delete_file_dir(dd->dd_dir);
 }
 
-static char *LoadTextFile(const char *pPath)
+static char *loadtxt_file(const char *pPath)
 {
     FILE *fp = fopen(pPath, "r");
     if (!fp)
@@ -426,7 +427,7 @@ static char *LoadTextFile(const char *pPath)
     return strbuf_free_nobuf(buf_content);
 }
 
-static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned size, uid_t
uid, gid_t gid)
+static bool savebin_file(const char *pPath, const char* pData, unsigned size, uid_t uid,
gid_t gid)
 {
     /* "Why 0640?!" See ::Create() for security analysis */
     unlink(pPath);
@@ -457,7 +458,7 @@ char* dd_loadtxt(const dump_dir_t *dd, const char *name)
         error_msg_and_die("DebugDump is not opened");
 
     char *full_path = concat_path_file(dd->dd_dir, name);
-    char *ret = LoadTextFile(full_path);
+    char *ret = loadtxt_file(full_path);
     free(full_path);
 
     return ret;
@@ -469,7 +470,7 @@ void dd_savetxt(dump_dir_t *dd, const char *name, const char *data)
         error_msg_and_die("DebugDump is not opened");
 
     char *full_path = concat_path_file(dd->dd_dir, name);
-    SaveBinaryFile(full_path, data, strlen(data), dd->uid, dd->gid);
+    savebin_file(full_path, data, strlen(data), dd->uid, dd->gid);
     free(full_path);
 }
 
@@ -479,7 +480,7 @@ void dd_savebin(dump_dir_t* dd, const char* name, const char* data,
unsigned siz
         error_msg_and_die("DebugDump is not opened");
 
     char *full_path = concat_path_file(dd->dd_dir, name);
-    SaveBinaryFile(full_path, data, size, dd->uid, dd->gid);
+    savebin_file(full_path, data, size, dd->uid, dd->gid);
     free(full_path);
 }
 
-- 
1.7.1

[PATCH 2/2] DebugDump.cpp: lower case variables

[Nikola Pajkovsky]

Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
---
 lib/utils/DebugDump.cpp |   72 +++++++++++++++++++++++-----------------------
 1 files changed, 36 insertions(+), 36 deletions(-)

diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
index 37e21c7..a96a4ee 100644
--- a/lib/utils/DebugDump.cpp
+++ b/lib/utils/DebugDump.cpp
@@ -35,18 +35,18 @@ static bool isdigit_str(const char *str)
 }
 
 // doesn't it only slash?
-static char* rm_backslashes(const char *pDir)
+static char* rm_backslashes(const char *dir)
 {
-    unsigned len = strlen(pDir);
-    while (len != 0 && pDir[len-1] == '/')
+    unsigned len = strlen(dir);
+    while (len != 0 && dir[len-1] == '/')
         len--;
-    return xstrndup(pDir, len);
+    return xstrndup(dir, len);
 }
 
-static bool exist_file_dir(const char *pPath)
+static bool exist_file_dir(const char *path)
 {
     struct stat buf;
-    if (stat(pPath, &buf) == 0)
+    if (stat(path, &buf) == 0)
     {
         if (S_ISDIR(buf.st_mode) || S_ISREG(buf.st_mode))
         {
@@ -115,30 +115,30 @@ int dd_exist(dump_dir_t *dd, const char *path)
     return ret;
 }
 
-static bool get_and_set_lock(const char* pLockFile, const char* pPID)
+static bool get_and_set_lock(const char* lock_file, const char* pid)
 {
-    while (symlink(pPID, pLockFile) != 0)
+    while (symlink(pid, lock_file) != 0)
     {
         if (errno != EEXIST)
-            perror_msg_and_die("Can't create lock file '%s'",
pLockFile);
+            perror_msg_and_die("Can't create lock file '%s'",
lock_file);
 
         char pid_buf[sizeof(pid_t)*3 + 4];
-        ssize_t r = readlink(pLockFile, pid_buf, sizeof(pid_buf) - 1);
+        ssize_t r = readlink(lock_file, pid_buf, sizeof(pid_buf) - 1);
         if (r < 0)
         {
             if (errno == ENOENT)
             {
-                /* Looks like pLockFile was deleted */
+                /* Looks like lock_file was deleted */
                 usleep(10 * 1000); /* avoid CPU eating loop */
                 continue;
             }
-            perror_msg_and_die("Can't read lock file '%s'",
pLockFile);
+            perror_msg_and_die("Can't read lock file '%s'",
lock_file);
         }
         pid_buf[r] = '\0';
 
-        if (strcmp(pid_buf, pPID) == 0)
+        if (strcmp(pid_buf, pid) == 0)
         {
-            log("Lock file '%s' is already locked by us", pLockFile);
+            log("Lock file '%s' is already locked by us", lock_file);
             return false;
         }
         if (isdigit_str(pid_buf))
@@ -147,19 +147,19 @@ static bool get_and_set_lock(const char* pLockFile, const char*
pPID)
             sprintf(pid_str, "/proc/%s", pid_buf);
             if (access(pid_str, F_OK) == 0)
             {
-                log("Lock file '%s' is locked by process %s",
pLockFile, pid_buf);
+                log("Lock file '%s' is locked by process %s",
lock_file, pid_buf);
                 return false;
             }
-            log("Lock file '%s' was locked by process %s, but it
crashed?", pLockFile, pid_buf);
+            log("Lock file '%s' was locked by process %s, but it
crashed?", lock_file, pid_buf);
         }
         /* The file may be deleted by now by other process. Ignore ENOENT */
-        if (unlink(pLockFile) != 0 && errno != ENOENT)
+        if (unlink(lock_file) != 0 && errno != ENOENT)
         {
-            perror_msg_and_die("Can't remove stale lock file '%s'",
pLockFile);
+            perror_msg_and_die("Can't remove stale lock file '%s'",
lock_file);
         }
     }
 
-    VERB1 log("Locked '%s'", pLockFile);
+    VERB1 log("Locked '%s'", lock_file);
     return true;
 
 #if 0
@@ -353,15 +353,15 @@ int dd_create(dump_dir_t *dd, const char *dir, uid_t uid)
     return 1;
 }
 
-static bool delete_file_dir(const char *pDir)
+static bool delete_file_dir(const char *dir)
 {
-    if (!exist_file_dir(pDir))
+    if (!exist_file_dir(dir))
         return true;
 
-    DIR *d = opendir(pDir);
+    DIR *d = opendir(dir);
     if (!d)
     {
-        error_msg("Can't open dir '%s'", pDir);
+        error_msg("Can't open dir '%s'", dir);
         return false;
     }
 
@@ -370,7 +370,7 @@ static bool delete_file_dir(const char *pDir)
     {
         if (dot_or_dotdot(dent->d_name))
             continue;
-        char *full_path = concat_path_file(pDir, dent->d_name);
+        char *full_path = concat_path_file(dir, dent->d_name);
         if (unlink(full_path) == -1)
         {
             if (errno != EISDIR)
@@ -385,9 +385,9 @@ static bool delete_file_dir(const char *pDir)
         free(full_path);
     }
     closedir(d);
-    if (rmdir(pDir) == -1)
+    if (rmdir(dir) == -1)
     {
-        error_msg("Can't remove dir %s", pDir);
+        error_msg("Can't remove dir %s", dir);
         return false;
     }
 
@@ -404,12 +404,12 @@ void dd_delete(dump_dir_t *dd)
     delete_file_dir(dd->dd_dir);
 }
 
-static char *loadtxt_file(const char *pPath)
+static char *loadtxt_file(const char *path)
 {
-    FILE *fp = fopen(pPath, "r");
+    FILE *fp = fopen(path, "r");
     if (!fp)
     {
-        perror_msg("Can't open file '%s'", pPath);
+        perror_msg("Can't open file '%s'", path);
         return xstrdup("");
     }
 
@@ -427,25 +427,25 @@ static char *loadtxt_file(const char *pPath)
     return strbuf_free_nobuf(buf_content);
 }
 
-static bool savebin_file(const char *pPath, const char* pData, unsigned size, uid_t uid,
gid_t gid)
+static bool savebin_file(const char *path, const char* data, unsigned size, uid_t uid,
gid_t gid)
 {
     /* "Why 0640?!" See ::Create() for security analysis */
-    unlink(pPath);
-    int fd = open(pPath, O_WRONLY | O_TRUNC | O_CREAT, 0640);
+    unlink(path);
+    int fd = open(path, O_WRONLY | O_TRUNC | O_CREAT, 0640);
     if (fd < 0)
     {
-        error_msg("Can't open file '%s': %s", pPath, errno ?
strerror(errno) : "errno == 0");
+        error_msg("Can't open file '%s': %s", path, errno ?
strerror(errno) : "errno == 0");
         return false;
     }
     if (fchown(fd, uid, gid) == -1)
     {
-        perror_msg("can't change '%s' ownership to %lu:%lu", pPath,
(long)uid, (long)gid);
+        perror_msg("can't change '%s' ownership to %lu:%lu", path,
(long)uid, (long)gid);
     }
-    unsigned r = full_write(fd, pData, size);
+    unsigned r = full_write(fd, data, size);
     close(fd);
     if (r != size)
     {
-        error_msg("Can't save file '%s'", pPath);
+        error_msg("Can't save file '%s'", path);
         return false;
     }
 
-- 
1.7.1

[PATCH 2/2] DebugDump.cpp: lower case variables

[Denys Vlasenko]

On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
>  ---
>   lib/utils/DebugDump.cpp |   72 +++++++++++++++++++++++-----------------------
>   1 files changed, 36 insertions(+), 36 deletions(-) 
Looks ok to me.
-- 
vda

[Denys Vlasenko]

On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
>  Signed-off-by: Nikola Pajkovsky <npajkovs(a)redhat.com&gt;
>  ---
>   lib/utils/DebugDump.cpp |   43 ++++++++++++++++++++++---------------------
>   1 files changed, 22 insertions(+), 21 deletions(-)
>  
>  diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
>  index 8987b43..37e21c7 100644
>  --- a/lib/utils/DebugDump.cpp
>  +++ b/lib/utils/DebugDump.cpp
>  @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
>       return true;
>   }
>   
>  -static char* RemoveBackSlashes(const char *pDir)
>  +// doesn't it only slash?
>  +static char* rm_backslashes(const char *pDir) 
You are right. It removes slashes, not backslashes.
I propose rm_trailing_slashes

>  -static bool ExistFileDir(const char *pPath)
>  +static bool exist_file_dir(const char *pPath) 
Ok.

>  -static char *LoadTextFile(const char *path);
>  +static char *loadtxt_file(const char *path); 
Why not load_text_file?

>  -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
>  +static bool get_and_set_lock(const char* pLockFile, const char* pPID) 
ok

>  -static bool DeleteFileDir(const char *pDir)
>  +static bool delete_file_dir(const char *pDir) 
Ok
 
>  -static bool SaveBinaryFile(const char *pPath, const char* pData,
> unsigned size, uid_t uid, gid_t gid)
>  +static bool savebin_file(const char *pPath, const char* pData, unsigned size, uid_t uid,
> gid_t gid) 
I propose save_binary_file

-- 
vda

[Nikola Pajkovsky]

On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
>  On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
> > Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> > ---
> >   lib/utils/DebugDump.cpp |   43 ++++++++++++++++++++++---------------------
> >   1 files changed, 22 insertions(+), 21 deletions(-)
> >
> > diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
> > index 8987b43..37e21c7 100644
> > --- a/lib/utils/DebugDump.cpp
> > +++ b/lib/utils/DebugDump.cpp
> > @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
> >       return true;
> >   }
> >
> > -static char* RemoveBackSlashes(const char *pDir)
> > +// doesn't it only slash?
> > +static char* rm_backslashes(const char *pDir)
>
>  You are right. It removes slashes, not backslashes.
>  I propose rm_trailing_slashes
>  
Sounds good to me.

> > -static bool ExistFileDir(const char *pPath)
> > +static bool exist_file_dir(const char *pPath)
>
>  Ok.
>
> > -static char *LoadTextFile(const char *path);
> > +static char *loadtxt_file(const char *path);
>
>  Why not load_text_file?
>  
we have dd_loadtxt so I wanted to have in same form, but I don't
have any problem with your proposal.

> > -static bool GetAndSetLock(const char* pLockFile, const char*
> pPID)
> > +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
>
>  ok
>
> > -static bool DeleteFileDir(const char *pDir)
> > +static bool delete_file_dir(const char *pDir)
>
>  Ok
>
> > -static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned size,
> uid_t uid, gid_t gid)
> > +static bool savebin_file(const char *pPath, const char* pData, unsigned size, uid_t
> uid, gid_t gid)
>
>  I propose save_binary_file
>  
ditto

[Jiri Moskovcak]

On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
>  On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
> > On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
> >> Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> >> ---
> >>   lib/utils/DebugDump.cpp |   43 ++++++++++++++++++++++---------------------
> >>   1 files changed, 22 insertions(+), 21 deletions(-)
> >>
> >> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
> >> index 8987b43..37e21c7 100644
> >> --- a/lib/utils/DebugDump.cpp
> >> +++ b/lib/utils/DebugDump.cpp
> >> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
> >>       return true;
> >>   }
> >>
> >> -static char* RemoveBackSlashes(const char *pDir)
> >> +// doesn't it only slash?
> >> +static char* rm_backslashes(const char *pDir)
> >
> > You are right. It removes slashes, not backslashes.
> > I propose rm_trailing_slashes
> >
>  
>  Sounds good to me.
>  
> >> -static bool ExistFileDir(const char *pPath)
> >> +static bool exist_file_dir(const char *pPath)
> >
> > Ok.
> >
> >> -static char *LoadTextFile(const char *path);
> >> +static char *loadtxt_file(const char *path);
> >
> > Why not load_text_file?
> >
>  
>  we have dd_loadtxt so I wanted to have in same form, but I don't
>  have any problem with your proposal.
>   
so the best would be to unify it and rename dd_loadtxt do dd_load_text...


> >> -static bool GetAndSetLock(const char* pLockFile, const char*
> pPID)
> >> +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
> >
> > ok
> >
> >> -static bool DeleteFileDir(const char *pDir)
> >> +static bool delete_file_dir(const char *pDir)
> >
> > Ok
> >
> >> -static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned size,
> uid_t uid, gid_t gid)
> >> +static bool savebin_file(const char *pPath, const char* pData, unsigned size,
> uid_t uid, gid_t gid)
> >
> > I propose save_binary_file
> >
>  
>  ditto
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Nikola Pajkovsky]

Jiri Moskovcak <jmoskovc(a)redhat.com&gt; wrote:

>  On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
>  > On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
>  >> On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
>  >>> Signed-off-by: Nikola Pajkovsky&lt;npajkovs(a)redhat.com&gt;
>  >>> ---
>  >>>   lib/utils/DebugDump.cpp |   43
> ++++++++++++++++++++++---------------------
>  >>>   1 files changed, 22 insertions(+), 21 deletions(-)
>  >>>
>  >>> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
>  >>> index 8987b43..37e21c7 100644
>  >>> --- a/lib/utils/DebugDump.cpp
>  >>> +++ b/lib/utils/DebugDump.cpp
>  >>> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
>  >>>       return true;
>  >>>   }
>  >>>
>  >>> -static char* RemoveBackSlashes(const char *pDir)
>  >>> +// doesn't it only slash?
>  >>> +static char* rm_backslashes(const char *pDir)
>  >>
>  >> You are right. It removes slashes, not backslashes.
>  >> I propose rm_trailing_slashes
>  >>
>  > 
>  > Sounds good to me.
>  > 
>  >>> -static bool ExistFileDir(const char *pPath)
>  >>> +static bool exist_file_dir(const char *pPath)
>  >>
>  >> Ok.
>  >>
>  >>> -static char *LoadTextFile(const char *path);
>  >>> +static char *loadtxt_file(const char *path);
>  >>
>  >> Why not load_text_file?
>  >>
>  > 
>  > we have dd_loadtxt so I wanted to have in same form, but I don't
>  > have any problem with your proposal.
>  > 
>
>  so the best would be to unify it and rename dd_loadtxt do dd_load_text...
>
>
>  >>> -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
>  >>> +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
>  >>
>  >> ok
>  >>
>  >>> -static bool DeleteFileDir(const char *pDir)
>  >>> +static bool delete_file_dir(const char *pDir)
>  >>
>  >> Ok
>  >>
>  >>> -static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
>  >>> +static bool savebin_file(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
>  >>
>  >> I propose save_binary_file
>  >>
>  > 
>  > ditto
>  > _______________________________________________
>  > Crash-catcher mailing list
>  > Crash-catcher(a)lists.fedorahosted.org
>  > https://fedorahosted.org/mailman/listinfo/crash-catcher
>
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Nikola Pajkovsky]

Jiri Moskovcak <jmoskovc(a)redhat.com&gt; wrote:

>  On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
>  > On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
>  >> On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
>  >>> Signed-off-by: Nikola Pajkovsky&lt;npajkovs(a)redhat.com&gt;
>  >>> ---
>  >>>   lib/utils/DebugDump.cpp |   43
> ++++++++++++++++++++++---------------------
>  >>>   1 files changed, 22 insertions(+), 21 deletions(-)
>  >>>
>  >>> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
>  >>> index 8987b43..37e21c7 100644
>  >>> --- a/lib/utils/DebugDump.cpp
>  >>> +++ b/lib/utils/DebugDump.cpp
>  >>> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
>  >>>       return true;
>  >>>   }
>  >>>
>  >>> -static char* RemoveBackSlashes(const char *pDir)
>  >>> +// doesn't it only slash?
>  >>> +static char* rm_backslashes(const char *pDir)
>  >>
>  >> You are right. It removes slashes, not backslashes.
>  >> I propose rm_trailing_slashes
>  >>
>  > 
>  > Sounds good to me.
>  > 
>  >>> -static bool ExistFileDir(const char *pPath)
>  >>> +static bool exist_file_dir(const char *pPath)
>  >>
>  >> Ok.
>  >>
>  >>> -static char *LoadTextFile(const char *path);
>  >>> +static char *loadtxt_file(const char *path);
>  >>
>  >> Why not load_text_file?
>  >>
>  > 
>  > we have dd_loadtxt so I wanted to have in same form, but I don't
>  > have any problem with your proposal.
>  > 
>
>  so the best would be to unify it and rename dd_loadtxt do dd_load_text...
>
>
>  >>> -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
>  >>> +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
>  >>
>  >> ok
>  >>
>  >>> -static bool DeleteFileDir(const char *pDir)
>  >>> +static bool delete_file_dir(const char *pDir)
>  >>
>  >> Ok
>  >>
>  >>> -static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
>  >>> +static bool savebin_file(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
>  >>
>  >> I propose save_binary_file
>  >>
>  > 
>  > ditto
>  > _______________________________________________
>  > Crash-catcher mailing list
>  > Crash-catcher(a)lists.fedorahosted.org
>  > https://fedorahosted.org/mailman/listinfo/crash-catcher
>
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher 



[Nikola Pajkovsky]

On 09/01/2010 02:05 PM, Jiri Moskovcak wrote:
>  On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
> > On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
> >> On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
> >>> Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> >>> ---
> >>>    lib/utils/DebugDump.cpp |   43
> ++++++++++++++++++++++---------------------
> >>>    1 files changed, 22 insertions(+), 21 deletions(-)
> >>>
> >>> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
> >>> index 8987b43..37e21c7 100644
> >>> --- a/lib/utils/DebugDump.cpp
> >>> +++ b/lib/utils/DebugDump.cpp
> >>> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
> >>>        return true;
> >>>    }
> >>>
> >>> -static char* RemoveBackSlashes(const char *pDir)
> >>> +// doesn't it only slash?
> >>> +static char* rm_backslashes(const char *pDir)
> >>
> >> You are right. It removes slashes, not backslashes.
> >> I propose rm_trailing_slashes
> >>
> >
> > Sounds good to me.
> >
> >>> -static bool ExistFileDir(const char *pPath)
> >>> +static bool exist_file_dir(const char *pPath)
> >>
> >> Ok.
> >>
> >>> -static char *LoadTextFile(const char *path);
> >>> +static char *loadtxt_file(const char *path);
> >>
> >> Why not load_text_file?
> >>
> >
> > we have dd_loadtxt so I wanted to have in same form, but I don't
> > have any problem with your proposal.
> >
>
>  so the best would be to unify it and rename dd_loadtxt do dd_load_text...
>  
this is not only dd_load_txt, but also dd_savetxt to dd_save_text(a little more work).
I have no problem with loadtxt_file, but if you have another opinion
I'm all ears[hate democracy:)]

> >>> -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
> >>> +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
> >>
> >> ok
> >>
> >>> -static bool DeleteFileDir(const char *pDir)
> >>> +static bool delete_file_dir(const char *pDir)
> >>
> >> Ok
> >>
> >>> -static bool SaveBinaryFile(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
> >>> +static bool savebin_file(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
> >>
> >> I propose save_binary_file
> >>
> >
> > ditto

[Jiri Moskovcak]

On 09/01/2010 02:43 PM, Nikola Pajkovsky wrote:
>  On 09/01/2010 02:05 PM, Jiri Moskovcak wrote:
> > On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
> >> On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
> >>> On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
> >>>> Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> >>>> ---
> >>>>    lib/utils/DebugDump.cpp |   43
> ++++++++++++++++++++++---------------------
> >>>>    1 files changed, 22 insertions(+), 21 deletions(-)
> >>>>
> >>>> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
> >>>> index 8987b43..37e21c7 100644
> >>>> --- a/lib/utils/DebugDump.cpp
> >>>> +++ b/lib/utils/DebugDump.cpp
> >>>> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
> >>>>        return true;
> >>>>    }
> >>>>
> >>>> -static char* RemoveBackSlashes(const char *pDir)
> >>>> +// doesn't it only slash?
> >>>> +static char* rm_backslashes(const char *pDir)
> >>>
> >>> You are right. It removes slashes, not backslashes.
> >>> I propose rm_trailing_slashes
> >>>
> >>
> >> Sounds good to me.
> >>
> >>>> -static bool ExistFileDir(const char *pPath)
> >>>> +static bool exist_file_dir(const char *pPath)
> >>>
> >>> Ok.
> >>>
> >>>> -static char *LoadTextFile(const char *path);
> >>>> +static char *loadtxt_file(const char *path);
> >>>
> >>> Why not load_text_file?
> >>>
> >>
> >> we have dd_loadtxt so I wanted to have in same form, but I don't
> >> have any problem with your proposal.
> >>
> >
> > so the best would be to unify it and rename dd_loadtxt do dd_load_text...
> >
>  
>  this is not only dd_load_txt, but also dd_savetxt to dd_save_text(a little more work).
>  I have no problem with loadtxt_file, but if you have another opinion
>  I'm all ears[hate democracy:)]
>   
by "unify it" I of course meant ALL of it... so the names will be:
dd_{load,save}_{bin,text}_file

> >
> >>>> -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
> >>>> +static bool get_and_set_lock(const char* pLockFile, const char* pPID)
> >>>
> >>> ok
> >>>
> >>>> -static bool DeleteFileDir(const char *pDir)
> >>>> +static bool delete_file_dir(const char *pDir)
> >>>
> >>> Ok
> >>>
> >>>> -static bool SaveBinaryFile(const char *pPath, const char* pData,
> unsigned size, uid_t uid, gid_t gid)
> >>>> +static bool savebin_file(const char *pPath, const char* pData, unsigned
> size, uid_t uid, gid_t gid)
> >>>
> >>> I propose save_binary_file
> >>>
> >>
> >> ditto
>  
>  _______________________________________________
>  Crash-catcher mailing list
>  Crash-catcher(a)lists.fedorahosted.org
>  https://fedorahosted.org/mailman/listinfo/crash-catcher

[Nikola Pajkovsky]

On 09/01/2010 02:46 PM, Jiri Moskovcak wrote:
>  On 09/01/2010 02:43 PM, Nikola Pajkovsky wrote:
> > On 09/01/2010 02:05 PM, Jiri Moskovcak wrote:
> >> On 09/01/2010 01:55 PM, Nikola Pajkovsky wrote:
> >>> On 09/01/2010 12:26 PM, Denys Vlasenko wrote:
> >>>> On Wed, 2010-09-01 at 11:46 +0200, Nikola Pajkovsky wrote:
> >>>>> Signed-off-by: Nikola Pajkovsky<npajkovs(a)redhat.com>
> >>>>> ---
> >>>>>     lib/utils/DebugDump.cpp |   43
> ++++++++++++++++++++++---------------------
> >>>>>     1 files changed, 22 insertions(+), 21 deletions(-)
> >>>>>
> >>>>> diff --git a/lib/utils/DebugDump.cpp b/lib/utils/DebugDump.cpp
> >>>>> index 8987b43..37e21c7 100644
> >>>>> --- a/lib/utils/DebugDump.cpp
> >>>>> +++ b/lib/utils/DebugDump.cpp
> >>>>> @@ -34,7 +34,8 @@ static bool isdigit_str(const char *str)
> >>>>>         return true;
> >>>>>     }
> >>>>>
> >>>>> -static char* RemoveBackSlashes(const char *pDir)
> >>>>> +// doesn't it only slash?
> >>>>> +static char* rm_backslashes(const char *pDir)
> >>>>
> >>>> You are right. It removes slashes, not backslashes.
> >>>> I propose rm_trailing_slashes
> >>>>
> >>>
> >>> Sounds good to me.
> >>>
> >>>>> -static bool ExistFileDir(const char *pPath)
> >>>>> +static bool exist_file_dir(const char *pPath)
> >>>>
> >>>> Ok.
> >>>>
> >>>>> -static char *LoadTextFile(const char *path);
> >>>>> +static char *loadtxt_file(const char *path);
> >>>>
> >>>> Why not load_text_file?
> >>>>
> >>>
> >>> we have dd_loadtxt so I wanted to have in same form, but I don't
> >>> have any problem with your proposal.
> >>>
> >>
> >> so the best would be to unify it and rename dd_loadtxt do dd_load_text...
> >>
> >
> > this is not only dd_load_txt, but also dd_savetxt to dd_save_text(a little more
> work).
> > I have no problem with loadtxt_file, but if you have another opinion
> > I'm all ears[hate democracy:)]
> >
>
>  by "unify it" I of course meant ALL of it... so the names will be:
>  dd_{load,save}_{bin,text}_file
>  
What do you want to use bin/txt or binary/text?

> >>
> >>>>> -static bool GetAndSetLock(const char* pLockFile, const char* pPID)
> >>>>> +static bool get_and_set_lock(const char* pLockFile, const char*
> pPID)
> >>>>
> >>>> ok
> >>>>
> >>>>> -static bool DeleteFileDir(const char *pDir)
> >>>>> +static bool delete_file_dir(const char *pDir)
> >>>>
> >>>> Ok
> >>>>
> >>>>> -static bool SaveBinaryFile(const char *pPath, const char* pData,
> unsigned size, uid_t uid, gid_t gid)
> >>>>> +static bool savebin_file(const char *pPath, const char* pData,
> unsigned size, uid_t uid, gid_t gid)
> >>>>
> >>>> I propose save_binary_file
> >>>>
> >>>
> >>> ditto
> >

[Denys Vlasenko]

On Wed, 2010-09-01 at 14:05 +0200, Jiri Moskovcak wrote:
>  >>> -static char *LoadTextFile(const char *path);
>  >>> +static char *loadtxt_file(const char *path);
>  >>
>  >> Why not load_text_file?
>  > 
>  > we have dd_loadtxt so I wanted to have in same form, but I don't
>  > have any problem with your proposal. 
I can explain a bit more:

"loadtxt_file" looks like a "verb_noun" construct.
Examples of such constructs: print_string, show_msgbox.
But "loadtxt" is not a verb - you cannot loadtxt anything.

load_txtfile would look a bit better.

But then someone may ask "why there is a space between load and txt
but not between txt and file?

And another problem with shortcuts: after some time you'll forget
where you need to use "txt" and where you need to use "text".
Therefore, shortcuts are good only when they save a lot,
like "dir" for "directory".

"txt" for "text" just does not worth it.

>  so the best would be to unify it and rename dd_loadtxt do
> dd_load_text... 
I agree.

-- 
vda