From owner-svn-src-all@freebsd.org Mon Jan 8 20:28:00 2018 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id D657EE5F8DA; Mon, 8 Jan 2018 20:28:00 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: from mail-io0-f170.google.com (mail-io0-f170.google.com [209.85.223.170]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id A65F3728F1; Mon, 8 Jan 2018 20:27:59 +0000 (UTC) (envelope-from cse.cem@gmail.com) Received: by mail-io0-f170.google.com with SMTP id z130so15986617ioe.13; Mon, 08 Jan 2018 12:27:59 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:reply-to:in-reply-to:references :from:date:message-id:subject:to:cc; bh=eicwRyrtt8nnK8urFqWz+x5fXTQF6E0niTcCr2g8N4c=; b=HIXlU8tRNQBpYHK1NTH9HlHQ6vOCelYQ4+zKcGs33iQGkCFnOgedh2yPKh64HukQmx Ly3CNt8t1pjKr7Zc48Dz7/kVbhsu+g1JvTsqsiobhSxFFK2rZS4SyezJ+VCopJG2Q67v uyp6pNYg8fEth4iy7OVoG9zky2r+bbrttfv6yDQkOB6f7manKpU9D/ZB9Dvaj7KR37LP HNF75N1LFlj0XbkTd/iuQJpFfRPVeNgtP46ZQvrUR1b1RwetsNQa3y+j5ZA+C5OKn9YI a3s2y88npZGwYoa0+AzQFqWmDc5seGIHpoxXRbTrGZ1EQtkk4JJGUMX0XxdUuhMNWPN7 sAbQ== X-Gm-Message-State: AKGB3mITyh1feL5Acfv9YFk0i3Rkq2EOc27LWTnDTKCU3ICHMmB6GRum /HAsLcMDmPa/bJmRO7t57yvlrRp5 X-Google-Smtp-Source: ACJfBosEL/M5T9/a+cQreHvPIOjVorAhaviVOm1sYK3JKgvJqYfDUL/VYiq/qt3pXqSe5UpidBfIAg== X-Received: by 10.107.180.201 with SMTP id d192mr12086813iof.270.1515443279018; Mon, 08 Jan 2018 12:27:59 -0800 (PST) Received: from mail-it0-f45.google.com (mail-it0-f45.google.com. [209.85.214.45]) by smtp.gmail.com with ESMTPSA id v19sm7566758ite.4.2018.01.08.12.27.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 08 Jan 2018 12:27:58 -0800 (PST) Received: by mail-it0-f45.google.com with SMTP id p124so1904745ite.1; Mon, 08 Jan 2018 12:27:58 -0800 (PST) X-Received: by 10.36.7.65 with SMTP id f62mr13307198itf.75.1515443278518; Mon, 08 Jan 2018 12:27:58 -0800 (PST) MIME-Version: 1.0 Reply-To: cem@freebsd.org Received: by 10.2.15.193 with HTTP; Mon, 8 Jan 2018 12:27:58 -0800 (PST) In-Reply-To: References: <201801081609.w08G9941022351@pdx.rh.CN85.dnsmgr.net> From: Conrad Meyer Date: Mon, 8 Jan 2018 12:27:58 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: svn commit: r327699 - head/sys/sys To: Pedro Giffuni Cc: src-committers , svn-src-all@freebsd.org, svn-src-head@freebsd.org Content-Type: text/plain; charset="UTF-8" X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.25 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 08 Jan 2018 20:28:00 -0000 I'm (again, atypically) with rgrimes here. Revert has a specific meaning and shouldn't be used like this. Making a commit with the subject "Revert rXXX" that does more than just "svn revert rXXX" makes life harder for developers looking at code history after you. Making two separate commits for two different changes (revert, then add the annotation) is not burdensome. Best, Conrad On Mon, Jan 8, 2018 at 12:18 PM, Pedro Giffuni wrote: > > On 08/01/2018 11:09, Rodney W. Grimes wrote: >>> >>> Author: pfg >>> Date: Mon Jan 8 15:54:29 2018 >>> New Revision: 327699 >>> URL: https://svnweb.freebsd.org/changeset/base/327699 >>> >>> Log: >>> Revert r327697: >>> malloc(9): drop the __result_use_check attribute for the kernel >>> allocator. >>> My bad: __result_use_check just checks the for the general and we >>> always >>> want to make sure allocated memory is used, not only checked for >>> nullness. >>> Add it to reallocf since that was missing. >> >> Please try not to combine a revert with an add, it makes it messy >> to try and figure out things in the future when only the svn log >> is being used to analyze stuff as digging in mail archives becomes >> painful. >> >> Revert, then commit the add standalone, is the better sequence in >> this type of situation. > > > Not that any of this is defined in our committers guide but IMHO "svn > revert" is just a tool, pretty much as "svn move" and "svn copy". The idea > is to make a committers' life easier: making two commits for such a simple > change is not "easier". > > In this case the change is rather consistent: I added __result_use_check to > the three functions that needed it. > The commit log is not only clear on why the revert happened but also > explains the additional one line change. > > When I MFC it, I will merge both changes for repository consistency but the > log will basically mention that I am adding __result_use_check to > reallocf(). > > > Pedro. > >>> Modified: >>> head/sys/sys/malloc.h >>> >>> Modified: head/sys/sys/malloc.h >>> >>> ============================================================================== >>> --- head/sys/sys/malloc.h Mon Jan 8 15:41:49 2018 (r327698) >>> +++ head/sys/sys/malloc.h Mon Jan 8 15:54:29 2018 (r327699) >>> @@ -176,7 +176,7 @@ void *contigmalloc(unsigned long size, struct >>> malloc_t >>> __alloc_size(1) __alloc_align(6); >>> void free(void *addr, struct malloc_type *type); >>> void *malloc(unsigned long size, struct malloc_type *type, int flags) >>> - __malloc_like __alloc_size(1); >>> + __malloc_like __result_use_check __alloc_size(1); >>> void *mallocarray(size_t nmemb, size_t size, struct malloc_type *type, >>> int flags) __malloc_like __result_use_check >>> __alloc_size(1) __alloc_size(2); >>> @@ -187,9 +187,9 @@ void malloc_type_freed(struct malloc_type >>> *type, unsig >>> void malloc_type_list(malloc_type_list_func_t *, void *); >>> void malloc_uninit(void *); >>> void *realloc(void *addr, unsigned long size, struct malloc_type >>> *type, >>> - int flags) __alloc_size(2); >>> + int flags) __result_use_check __alloc_size(2); >>> void *reallocf(void *addr, unsigned long size, struct malloc_type >>> *type, >>> - int flags) __alloc_size(2); >>> + int flags) __result_use_check __alloc_size(2); >>> struct malloc_type *malloc_desc2type(const char *desc); >>> #endif /* _KERNEL */ >>> >>> > > Pedro. >