From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-f41.google.com (mail-lf1-f41.google.com [209.85.167.41]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3588945C304 for ; Sun, 20 Dec 2020 19:58:44 +0300 (MSK) Received: by mail-lf1-f41.google.com with SMTP id a12so18069337lfl.6 for ; Sun, 20 Dec 2020 08:58:44 -0800 (PST) Date: Sun, 20 Dec 2020 19:58:41 +0300 From: Cyrill Gorcunov Message-ID: <20201220165841.GB3139@grain> References: <20201210161832.729439-1-gorcunov@gmail.com> <20201210161832.729439-4-gorcunov@gmail.com> <2a91ad22-6cd3-0bdd-78ef-203bd4b48a8d@tarantool.org> <20201215081645.GB983198@grain> <20201220154914.GA3139@grain> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH v4 3/4] crash: move fatal signal handling in List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladislav Shpilevoy Cc: Mons Anderson , tml On Sun, Dec 20, 2020 at 05:07:14PM +0100, Vladislav Shpilevoy wrote: > > > > I can simply drop the comment if you dont like it. > > No. I am trying to understand why one plain address matters, > and the others are not. But now I think I understood, thanks. > Keep the comment. OK. > >> commit. And this commit is not atomic. Also I know when a patch is > >> easy to follow and easy to review. This one isn't. Because I constantly > >> need to look for changes you did among hundreds of lines of refactoring. > >> > >> Please, split the independent changes into separate commits so as they > >> could be properly reviewed and the changes could be justified in the > >> commit messages. > > > > Currently the commit is big enough because I jump into signal handling > > in one pass. If you prefer I can split the series in more small changes. > > Or you mean the updates over previsous series should be changes in small > > steps as a separate commits? > > No, I don't mean commits with review fixes. I mean the patch commits. > Currently you have a patch that moves signal handling to crash.c AND it > also changes backtrace to use a 4KB buffer instead of the static buffer. Yes, this is my bad :-( I'm really sorry for that, Vlad! Actually I found it only after I've sent it out. I really appreciate all your comments! > These 2 changes (move and backtrace buffer source change) are not related. > Therefore it is not correct to put them into 1 commit. It does not matter > in which order you will do these 2 changes, but one should be about move > only, and the other one about backtrace buffer update only.