Static analysis issues (mostly memleaks) now fixed

[jrybar-rh]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 96

• 0 of 4 (0.0%) changed or added relevant lines in 2 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.02%) to 42.218%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
misc.c	0	1	0.0%
arg.c	0	3	0.0%

Totals
Change from base Build 90:	-0.02%
Covered Lines:	2631
Relevant Lines:	6232

---

💛 - Coveralls

[masatake]

The depth of indentation looks bad.
At a quick glance, ec is freed after returning from this function.
The indentation for free should not align with "return (1)".
It should be one level left.

[jrybar-rh]

Do you know why the indentation is messed up?
Because there are spaces before 'if' and tab before 'return'!
My god.
Then my editor's autoindent naturally copied the tab.

We need to be careful about this. Entire code of lsof is full of this mess.

[masatake]

The indentation level of the second free should be the same as the first free.

[jrybar-rh]

and here are spaces and tabs combined on one line (1214). I'll be damned.
Sorry. I'll take care of this, but I'm sure you can see this mistake was not intended and was hard to prevent.

[masatake]

Ditto.

[masatake]

Ditto.

[masatake]

I think this change of indentation level is not needed.

[masatake]

The line "return (1);" is nothing to do with the memleaks.
So, please, keep the line as the same as before.

[jrybar-rh]

No, return (1) has nothing to do with memleaks, but Readlink(ec) has.
On line 754 new string for ec is allocated. On line 762 Readlink returns new allocated string in some cases. Then ec is not used anymore and never freed.
For your sake the code of Readlink is so nasty that there are cases it returns ec back again.
So yeah, after finding (!!!) and peeking at Readlink() more thoroughly I concur ec cannot be freed, however there will be cases it WILL leak.

I will remove this part in next commit.

[masatake]

Plesae, don't mix whitespace related changes into this pull request.
As you wrote in the subject(?), this is for fixing memleaks.
Please, don't change the level of indentation. In other words, minimize the lines of diff.

I squashed your change into one patch:

[yamato@master]~/var/lsof-linux% cat your.patch
commit 8f7387db9d40f65dbd250df5d279e3a522313b86
Author: Jan Rybar <jrybar@redhat.com>
Date:   Tue Oct 9 18:52:10 2018 +0200

    Static analysis issues (mostly memleaks) now fixed

diff --git a/arg.c b/arg.c
index 5371927..7afff29 100644
--- a/arg.c
+++ b/arg.c
@@ -760,7 +760,7 @@ enter_efsys(e, rdlnk)
 	    path = ec;
 	else {
 	    if (!(path = Readlink(ec)))
-		return(1);
+	        return(1);
 	}
 /*
  * Remove terminating `/' characters from paths longer than one.
@@ -772,8 +772,10 @@ enter_efsys(e, rdlnk)
  * Enter file system path on list, avoiding duplicates.
  */
 	for (ep = Efsysl; ep; ep = ep->next) {
-	   if (!strcmp(ep->path, path))
-		return(0);
+	   if (!strcmp(ep->path, path)) {
+		   (void)free((FREE_P *)path);
+		   return (0);
+	   }
 	}
 	if (!(ep = (efsys_list_t *)malloc((MALLOC_S)(sizeof(efsys_list_t))))) {
 	   (void) fprintf(stderr, "%s: no space for \"-e %s\" entry\n",
diff --git a/misc.c b/misc.c
index 1682797..1b2c2ef 100644
--- a/misc.c
+++ b/misc.c
@@ -1212,8 +1212,10 @@ no_readlink_space:
 		stk[i] = (char *)NULL;
 	    }
 	    (void) free((FREE_P *)stk);
+	    (void) free((FREE_P *)s1);
 	    stk = (char **)NULL;
 	    ss = sx = 0;
+	    s1 = (char *)NULL;
 	    op = (char *)NULL;
 	    return((char *)NULL);
 	}

Look at the first "return (1);":

-		return(1);
+	        return(1);

I think this change has nothing to do with memleak. Am I wrong?

-	   if (!strcmp(ep->path, path))
-		return(0);
+	   if (!strcmp(ep->path, path)) {
+		   (void)free((FREE_P *)path);
+		   return (0);
+	   }

As far as other areas of lsof, following will be better:

-	   if (!strcmp(ep->path, path))
-		return(0);
+	   if (!strcmp(ep->path, path)) {
+		(void)free((FREE_P *)path);
+		return (0);
+	   }

GNU Emacs has a mode visualizing the whitespace. The mode tells us what happens in your change:

The 8 spaces in front of "return(1);" should be a tab.
The 3 spaces in front of "(void)free((FREE_P*)path);" should be removed.
The 3 spaces in front of " return (0);" should be removed.