g.gisenv: Fix Resource Leak issue in main.c

[ShubhamDesai]

This pull request fixes issue identified by Coverity Scan (CID : 1207681)
Used G_free() to fix this issue.

[ShubhamDesai]

@nilason won't the G_free directly work here?

[nilason]

@nilason won't the G_free directly work here?

Not quite sure what you mean.

[nilason]

@nilason won't the G_free directly work here?

Not quite sure what you mean.

If you mean G_free(name) directly after u_name = G_store(name);, then yes.

[ShubhamDesai]

@nilason won't the G_free directly work here?

Not quite sure what you mean.

If you mean G_free(name) directly after u_name = G_store(name);, then yes.

I used G_free() directly but still it failed.
why this issue arised? can you please help.

[nilason]

@nilason won't the G_free directly work here?

Not quite sure what you mean.

If you mean G_free(name) directly after u_name = G_store(name);, then yes.

I used G_free() directly but still it failed. why this issue arised? can you please help.

I now see you mean the tests are failing. The reason wasn't obvious to me at first. This may be a good exercise :).
Hint: understand the purpose of the function, and study the function line-by-line, in particular the use of the variable value, therein lies the root of the problem.

[ShubhamDesai]

@nilason won't the G_free directly work here?

Not quite sure what you mean.

If you mean G_free(name) directly after u_name = G_store(name);, then yes.

I used G_free() directly but still it failed. why this issue arised? can you please help.

I now see you mean the tests are failing. The reason wasn't obvious to me at first. This may be a good exercise :). Hint: understand the purpose of the function, and study the function line-by-line, in particular the use of the variable value, therein lies the root of the problem.

The parse_variable function parses a string such as "VAR=value", separating the variable name (VAR) and value (value). Based on your hint I updated the function and tested locally with following testcases
Input with a variable and value:
Input: "VAR=value"
Result: "VAR"
Value: "value"

Input with only a variable (no =):
Input: "VAR"
Result: "VAR"
Value: (null)

Input with an empty value:
Input: "VAR="
Result: "VAR"
Value: (null)

All of them were as expected. It passed on my local windows machine
Could you please review once.

[metzm]

Looks good to me, but please check with valgrind that there are no invalid memory read/write attempts.

[metzm]

Correction: does not look good to me, the strings stored in char *value by parse_variable() are not G_free()d at the end in main(). First, if there would be so many GRASS envs, that string their values in memory could lead to OOM errors, this fix would not help. Second, there is only a very limited number of GRASS envs, memory consumption by parse_variable() storing variable + value in name should not even be measurable on a current system. IMHO this fix does not make sense, I would leave the code as it is.

[metzm]

In short: storing data somewhere else without free'ing them is not a solution, and there is not much to free anyway, the system can take care of that.

[ShubhamDesai]

@nilason can you review once and tell me what is wrong.

[nilason]

Correction: does not look good to me, the strings stored in char *value by parse_variable() are not G_free()d at the end in main(). First, if there would be so many GRASS envs, that string their values in memory could lead to OOM errors, this fix would not help. Second, there is only a very limited number of GRASS envs, memory consumption by parse_variable() storing variable + value in name should not even be measurable on a current system. IMHO this fix does not make sense, I would leave the code as it is.

In short: storing data somewhere else without free'ing them is not a solution, and there is not much to free anyway, the system can take care of that.

It is in this case, as probably in many of the other 600+ resource leak issues reported by Coverity Scan, indeed more of a "false positive". At least, Coverity Scan does not report (correctly) on unfreed memory in main functions or immediately before an exit (as with G_fatal_error). I'm of the opinion that it is advantageous to limit the issues of tools such as Coverity Scan raises to be able to catch also the "real" ones. Easy and uncontroversial fixes, even if not critical, should be made for that reason alone.