From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DB0CB347FE1 for ; Fri, 19 Jun 2026 09:15:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.45 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781860533; cv=none; b=mc6K0qRJx8trIbJJaqTRhyo9fTcNg5mzQpDbbhoyxDiFe9ymnEf5P6hoNwmFXqbaObQulh9MsWAZvadwRAYtdr2vcee0TM2sU67tKsWv5/qT0QfbBdh/qsM6RhYsb64DIKzcvfUUonx1QvrS8cQWKu7U0jSWLapADDro/SIyK2g= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781860533; c=relaxed/simple; bh=ToYjuqrLUN40Tdb6VL9Do4hGOmu/TUx1TU/0PRrGPcg=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=BVT1St9XSs1hR2MOp8FlDBGc8wpgmYqqQLdRGCb+Gf+CDwlrv6X/jxI/CwXwtdk8HCoOvX1WX+ibg2QAH0S/KMRpmX8AMrQC9z3ZjUgL1ongpl2XZjDpYobXnCRm/gQ1tlobYNFvne54HpiOOpxC1N01Dl69Qb6Mibc+G4Y/t+s= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=azX3E7vx; arc=none smtp.client-ip=209.85.221.45 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="azX3E7vx" Received: by mail-wr1-f45.google.com with SMTP id ffacd0b85a97d-45fd461e4a5so1481048f8f.0 for ; Fri, 19 Jun 2026 02:15:31 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1781860530; x=1782465330; darn=vger.kernel.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=redxzYQzLRKG52vJq98WS9ectvhmVOA5UgA7NvGPOFo=; b=azX3E7vxPgvinqhnJ9TQ8WHMkfJG4OpMnr8/v35JKnWuUBbj/ZjLaTKoGMmrT1SOdd A2X2Npe4X1mIL31Qz0kaKCYqlMgf2sTMqEkhUDHkV1C7pXMmyMqtSTB4xoJZ3QdeIg7Q tvKXGOc4fwNuO2mAmbSM5Ng8XpsjZvjsr6mmdE5B4pebi/BUjOCB1hbytW95o3rH0pa5 FKxNKfnQaMBK65biiCRyZWN5Gy9QK9E8uZDFrM4N49dtqxLDUXmQrvcXvSHGDqgDfXbq 1iL1mOcRB9/pblLXfJAoBKMespZ90CkqrwNlfqUCXwsuzSjX7UHJ2SgDlix/Xw30r9G7 3FYg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1781860530; x=1782465330; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=redxzYQzLRKG52vJq98WS9ectvhmVOA5UgA7NvGPOFo=; b=UtrFY11E3j59kNN1h2iIJ5P0mCOuGTf9H/b8LkwzZ4IG/hgjnAKG+Y5eIKAuy5rECy /JQ/tQRBy5sVFtR8WLywP4Xb1As4tetWP+4aoLb6Tg5a6yDw7mIB1IdW4m5kNcPUZgxU RABu52efl/MBq37t9kPmlJXXdvOZ8eYBuX9YXfnP7mRa28XkACMGNhSt9Ot/PEkaSjb2 suBaP2VXZrbu1QlGGhxsWE4wRoy+Tyv0OU4EZK6EEhivmXOWzigkN2A6/H4qo59D8PHP 1VDNcvKzPPxNGSriAS44g2DXEmvba8DhfhnnIoSI0fOPNr/1PnS5PIkN7LsBfror0X6j j+GQ== X-Forwarded-Encrypted: i=1; AFNElJ9dyevhd/7nVRCRLAOWc/rhBPh8iA7utPjOjDlrny810zfRCUqtBCi4jt2E1voGsPkQ5Ob2yfRtKyJyOqE=@vger.kernel.org X-Gm-Message-State: AOJu0Yw7eAQL6UHXy4V080yJXqP8xLsH+bUPEh10/SrYT+IJFk2ppJs5 Pu9Ze/z1wV+7sKkVFTEAcUlAHNZM7iJXfdoV433b4suVCraiprjJIMWg X-Gm-Gg: AfdE7ck1Yvk258zqk0xEVke4TAYih2wtbmedZ3JtcmGDlEV19EVJi9N+tXHYEHg1RWz vnmsZVp/9jhrLFnsGKNtYwinlDffPVZje34PYcguH/ViUarn/mDIbfrbjtfVmOqDPrJVG0oEF4/ iOzFDWxqqpK3qqBwyAFkHhD13z4qQSoZ63AtQ1ksvUkMGLg79yeBB9dynUPNxcxY3iEAcaSrCY1 HEr3bDoenfR0DtQqV6SohvRWG2Exc3NrKEsmKBlrw7SEWxJcUSgzlVKotv4gLy669iXi4LlxYhJ XB7TwKVfTJG8RWf1h1VUo0yJNAGaxgtxJB9Buw4UQpo5rGY3XWDqveEQOVpNDBz3ayW0pAXZdNQ DnuqTupVICYD4CJwvZDSu3R4hfLpAbRFQhwccpghT6bxM+bi6YsYBBZiEFXr9YtRxctMz0B6BB/ Mczzxb X-Received: by 2002:a05:6000:605:b0:45e:ec27:b4b0 with SMTP id ffacd0b85a97d-4656c792222mr2571640f8f.18.1781860530015; Fri, 19 Jun 2026 02:15:30 -0700 (PDT) Received: from nsa ([148.63.225.166]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-46508a04c15sm6484050f8f.3.2026.06.19.02.15.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 19 Jun 2026 02:15:29 -0700 (PDT) Date: Fri, 19 Jun 2026 10:16:31 +0100 From: Nuno =?utf-8?B?U8Oh?= To: Rodrigo Alencar <455.rodrigo.alencar@gmail.com> Cc: rodrigo.alencar@analog.com, linux-iio@vger.kernel.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, linux-doc@vger.kernel.org, linux-hardening@vger.kernel.org, Lars-Peter Clausen , Michael Hennerich , Jonathan Cameron , David Lechner , Andy Shevchenko , Rob Herring , Krzysztof Kozlowski , Conor Dooley , Philipp Zabel , Jonathan Corbet , Shuah Khan , Kees Cook , "Gustavo A. R. Silva" Subject: Re: [PATCH v6 06/16] iio: core: create local __iio_chan_prefix_emit() for reuse Message-ID: References: <20260618-ad9910-iio-driver-v6-0-79125ffbe430@analog.com> <20260618-ad9910-iio-driver-v6-6-79125ffbe430@analog.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: On Thu, Jun 18, 2026 at 05:14:19PM +0100, Rodrigo Alencar wrote: > On 18/06/26 16:06, Nuno Sá wrote: > > On Thu, Jun 18, 2026 at 02:27:22PM +0100, Rodrigo Alencar via B4 Relay wrote: > > > From: Rodrigo Alencar > > > > > > Move logic to create a channel prefix for naming attribute files into a > > > separate __iio_chan_prefix_emit() function for reuse. > > ... > > > > +static int __iio_chan_prefix_emit(const struct iio_chan_spec *chan, > > > + enum iio_shared_by shared_by, > > > + char *buf, size_t len) > > > +{ > > > + const char *dir = iio_direction[chan->output]; > > > + const char *type = iio_chan_type_name_spec[chan->type]; > > > + int n = 0; > > > + > > > + switch (shared_by) { > > > + case IIO_SHARED_BY_ALL: > > > + buf[0] = '\0'; /* empty channel prefix */ > > > + break; > > > + case IIO_SHARED_BY_DIR: > > > + n = scnprintf(buf, len, "%s", dir); > > > + break; > > > + case IIO_SHARED_BY_TYPE: > > > + n = scnprintf(buf, len, "%s_%s", dir, type); > > > + if (chan->differential) > > > + n += scnprintf(buf + n, len - n, "-%s", type); > > > + break; > > > + case IIO_SEPARATE: > > > + if (chan->indexed) { > > > + n = scnprintf(buf, len, "%s_%s%d", dir, type, > > > + chan->channel); > > > + if (chan->differential) > > > + n += scnprintf(buf + n, len - n, "-%s%d", type, > > > + chan->channel2); > > > + } else { > > > + if (chan->differential) { > > > + WARN(1, "Differential channels must be indexed\n"); > > > + return -EINVAL; > > > + } > > > + n = scnprintf(buf, len, "%s_%s", dir, type); > > > + } > > > + > > > + if (chan->modified) { > > > + if (chan->differential) { > > > + WARN(1, "Differential channels can not have modifier\n"); > > > + return -EINVAL; > > > > WARN() looks too much to me. dev_error() as we're treating it as such. I > > guess you don't want to pass struct device but not really an issue IMHO. > > __iio_device_attr_init() also used WARN(), probably because it didnt have > access to a dev pointer. It would not be a problem to add an extra param. Hmm, fair enough. Maybe a chance to change it. Not sure how others feel about it. > > > > > > + } > > > + n += scnprintf(buf + n, len - n, "_%s", > > > + iio_modifier_names[chan->channel2]); > > > + } > > > + > > > + if (chan->extend_name) > > > + n += scnprintf(buf + n, len - n, "_%s", chan->extend_name); > > > + break; > > > + } > > > + > > > + if (n > 0 && n < len - 1) { /* prefix termination if not empty */ > > > + buf[n++] = '_'; > > > + buf[n] = '\0'; > > > + } > > > + > > > > Can't we handle the above in the caller on kasprintf()? Then we could > > simplify and return in place. > > I felt like doing this here would get a cleaner logic in the caller, which > would have to add the '_' conditionally. > I think it makes things more clear in the caller given we return n anyways but I don't feel strong about it. - Nuno Sá > > > > > + return n; > > > +} > > > + > > > /** > > > * iio_device_id() - query the unique ID for the device > > > * @indio_dev: Device structure whose ID is being queried > > > @@ -1100,106 +1159,19 @@ int __iio_device_attr_init(struct device_attribute *dev_attr, > > > size_t len), > > > enum iio_shared_by shared_by) > > > { > > > - int ret = 0; > > > - char *name = NULL; > > > - char *full_postfix; > > > + char prefix[NAME_MAX + 1]; > > > + int ret; > > > > > > sysfs_attr_init(&dev_attr->attr); > > > > > > - /* Build up postfix of __postfix */ > > > - if (chan->modified && (shared_by == IIO_SEPARATE)) { > > > - if (chan->extend_name) > > > - full_postfix = kasprintf(GFP_KERNEL, "%s_%s_%s", > > > - iio_modifier_names[chan->channel2], > > > - chan->extend_name, > > > - postfix); > > > - else > > > - full_postfix = kasprintf(GFP_KERNEL, "%s_%s", > > > - iio_modifier_names[chan->channel2], > > > - postfix); > > > - } else { > > > - if (chan->extend_name == NULL || shared_by != IIO_SEPARATE) > > > - full_postfix = kstrdup(postfix, GFP_KERNEL); > > > - else > > > - full_postfix = kasprintf(GFP_KERNEL, > > > - "%s_%s", > > > - chan->extend_name, > > > - postfix); > > > - } > > > - if (full_postfix == NULL) > > > + ret = __iio_chan_prefix_emit(chan, shared_by, prefix, sizeof(prefix)); > > > + if (ret < 0) > > > + return ret; > > > + > > > + dev_attr->attr.name = kasprintf(GFP_KERNEL, "%s%s", prefix, postfix); > > > + if (!dev_attr->attr.name) > > > return -ENOMEM; > > > > I don't oppose the change. Looks like a nice cleanup. But bear in mind > > this very sensible as any subtle mistake means ABI breakage. > > Yes! I tried to be careful... this is dangerous stuff! > > -- > Kind regards, > > Rodrigo Alencar