[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [master] Introduces CHECK_ASPRINTF macro that checks asprintfs return value and terminates program in OOM scenarios.

On 11/18/2009 04:37 AM, Ales Kozumplik wrote:

> Martin is right. I tried looking at a cpp output and the macros are
> indeed expanded into one line. Besides I tried compiling a small testing
> file with a macro, made it fail and observed that the line was reported
> exactly right.

Hrm.  It appears to do the correct thing with gcc-4.4.1.  I'm pretty
sure it's done the other way in the past, and that it's not actually
guaranteed by the standard, but I'll let that go.

> I have already replaced the macro name with lowercase version and I am
> going to build anaconda (once it is possible for rawhide) and check that
> the loader still works as David suggested.


> On a higher level: I despise macros as much as you do.

No, I don't think you do.

> I agree that they decrease the code quality. In our situation
> however, where the code has over 70 places where the same four lines
> exactly repeat, macros are the lesser evil. Have you noticed that on
> most places before the patch the log call looked like this:
> logMessage(CRITICAL, "%s: %d: %m", __func__, __LINE__);
> There is a new line missing, a failure would dump something like:
> critical main: 26: SuccessAborted

There is not a newline missing.  logMessage() automatically includes a
newline; putting one in the format specifier is the bug, not the other
way around.

> Or, consider that the logging mechanism will have to change for some
> reason and instead of CRITICAL we will have to write LOG_CRITICAL. This
> would be really annoying for someone to fix at all those places. That's
> exactly why I think we are trading some less-than-ideal practice for
> lower code duplicity and higher maintainability here.

This is a total strawman.  "%s/CRITICAL/LOG_CRITICAL/g" is not harder than

Look, if you really think it helps, go ahead.  I personally don't think
it does, but I also don't think it's such a big mistake as to be the end
of the world or something.


If the facts don't fit the theory, change the facts.
		-- Einstein

[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]