[PATCH v1 1/1] efi_loader: Mark a function static

[PATCH v1 1/1] efi_loader: Mark a function static

[Andy Shevchenko]

efi_bootmgr_release_uridp_resource() is not used anywhere except
the same file where it is defined. Mark it static.
This helps avoiding the compiler warning:

  lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 lib/efi_loader/efi_bootmgr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
index 589d3996b680..79144962cd73 100644
--- a/lib/efi_loader/efi_bootmgr.c
+++ b/lib/efi_loader/efi_bootmgr.c
@@ -385,7 +385,7 @@ err:
  * @ctx:	event context
  * Return:	status code
  */
-efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
+static efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
 {
 	efi_status_t ret = EFI_SUCCESS;
 
-- 
2.43.0.rc1.1336.g36b5255a03ac

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Ilias Apalodimas]

Thanks Andy

On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
>
> efi_bootmgr_release_uridp_resource() is not used anywhere except
> the same file where it is defined. Mark it static.
> This helps avoiding the compiler warning:
>
>   lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
>
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  lib/efi_loader/efi_bootmgr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
> index 589d3996b680..79144962cd73 100644
> --- a/lib/efi_loader/efi_bootmgr.c
> +++ b/lib/efi_loader/efi_bootmgr.c
> @@ -385,7 +385,7 @@ err:
>   * @ctx:       event context
>   * Return:     status code
>   */
> -efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
> +static efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
>  {
>         efi_status_t ret = EFI_SUCCESS;
>
> --
> 2.43.0.rc1.1336.g36b5255a03ac
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Heinrich Schuchardt]

On 10/21/24 16:40, Ilias Apalodimas wrote:
> Thanks Andy
>
> On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
> <andriy.shevchenko@linux.intel.com> wrote:
>>
>> efi_bootmgr_release_uridp_resource() is not used anywhere except
>> the same file where it is defined. Mark it static.
>> This helps avoiding the compiler warning:
>>
>>    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
>>
>> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
>> ---
>>   lib/efi_loader/efi_bootmgr.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_bootmgr.c b/lib/efi_loader/efi_bootmgr.c
>> index 589d3996b680..79144962cd73 100644
>> --- a/lib/efi_loader/efi_bootmgr.c
>> +++ b/lib/efi_loader/efi_bootmgr.c
>> @@ -385,7 +385,7 @@ err:
>>    * @ctx:       event context
>>    * Return:     status code
>>    */
>> -efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
>> +static efi_status_t efi_bootmgr_release_uridp_resource(struct uridp_context *ctx)
>>   {
>>          efi_status_t ret = EFI_SUCCESS;
>>
>> --
>> 2.43.0.rc1.1336.g36b5255a03ac
>>
>
> Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>

Hello Andy,

The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
("efi_loader: shorten efi_bootmgr_release_uridp_resource()").

Best regards

Heinrich

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Andy Shevchenko]

On Tue, Oct 22, 2024 at 08:02:46AM +0200, Heinrich Schuchardt wrote:
> On 10/21/24 16:40, Ilias Apalodimas wrote:
> > On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
> > <andriy.shevchenko@linux.intel.com> wrote:
> > > 
> > > efi_bootmgr_release_uridp_resource() is not used anywhere except
> > > the same file where it is defined. Mark it static.
> > > This helps avoiding the compiler warning:
> > > 
> > >    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]

> The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
> ("efi_loader: shorten efi_bootmgr_release_uridp_resource()").

Thanks! The problem is that U-Boot doesn't have the latest tag (yet)
that includes this change. You can help with managing the conflict.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Heinrich Schuchardt]

Am 22. Oktober 2024 15:18:45 MESZ schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
>On Tue, Oct 22, 2024 at 08:02:46AM +0200, Heinrich Schuchardt wrote:
>> On 10/21/24 16:40, Ilias Apalodimas wrote:
>> > On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
>> > <andriy.shevchenko@linux.intel.com> wrote:
>> > > 
>> > > efi_bootmgr_release_uridp_resource() is not used anywhere except
>> > > the same file where it is defined. Mark it static.
>> > > This helps avoiding the compiler warning:
>> > > 
>> > >    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
>
>> The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
>> ("efi_loader: shorten efi_bootmgr_release_uridp_resource()").
>
>Thanks! The problem is that U-Boot doesn't have the latest tag (yet)
>that includes this change. You can help with managing the conflict.
>


Patches should be based on origin/master (or origin/next once that branch is opened typically after -rc2) and not on tags.

Best regards

Heinrich

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Andy Shevchenko]

On Thu, Oct 24, 2024 at 07:03:24AM +0200, Heinrich Schuchardt wrote:
> Am 22. Oktober 2024 15:18:45 MESZ schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> >On Tue, Oct 22, 2024 at 08:02:46AM +0200, Heinrich Schuchardt wrote:
> >> On 10/21/24 16:40, Ilias Apalodimas wrote:
> >> > On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
> >> > <andriy.shevchenko@linux.intel.com> wrote:
> >> > > 
> >> > > efi_bootmgr_release_uridp_resource() is not used anywhere except
> >> > > the same file where it is defined. Mark it static.
> >> > > This helps avoiding the compiler warning:
> >> > > 
> >> > >    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
> >
> >> The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
> >> ("efi_loader: shorten efi_bootmgr_release_uridp_resource()").
> >
> >Thanks! The problem is that U-Boot doesn't have the latest tag (yet)
> >that includes this change. You can help with managing the conflict.
> 
> Patches should be based on origin/master (or origin/next once that branch is
> opened typically after -rc2) and not on tags.

I disagree. The problem with moving target that it's been moving...

The tags are very good to follow and easy to maintain and test and report
regressions against. What you are suggesting it's like virtually assigning
tag to very each commit and tell maintainer to cope with this chaos when
one does something in one "tag" out of 100500 ones and another person in
another "tag". So, consider tags as stabilisation points, or points of
stability. Then it's much easier to stick with a few tags that with 100500
commits.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Tom Rini]

[-- Attachment #1: Type: text/plain, Size: 2239 bytes --]

On Thu, Oct 24, 2024 at 09:26:14AM +0300, Andy Shevchenko wrote:
> On Thu, Oct 24, 2024 at 07:03:24AM +0200, Heinrich Schuchardt wrote:
> > Am 22. Oktober 2024 15:18:45 MESZ schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > >On Tue, Oct 22, 2024 at 08:02:46AM +0200, Heinrich Schuchardt wrote:
> > >> On 10/21/24 16:40, Ilias Apalodimas wrote:
> > >> > On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
> > >> > <andriy.shevchenko@linux.intel.com> wrote:
> > >> > > 
> > >> > > efi_bootmgr_release_uridp_resource() is not used anywhere except
> > >> > > the same file where it is defined. Mark it static.
> > >> > > This helps avoiding the compiler warning:
> > >> > > 
> > >> > >    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
> > >
> > >> The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
> > >> ("efi_loader: shorten efi_bootmgr_release_uridp_resource()").
> > >
> > >Thanks! The problem is that U-Boot doesn't have the latest tag (yet)
> > >that includes this change. You can help with managing the conflict.
> > 
> > Patches should be based on origin/master (or origin/next once that branch is
> > opened typically after -rc2) and not on tags.
> 
> I disagree. The problem with moving target that it's been moving...
> 
> The tags are very good to follow and easy to maintain and test and report
> regressions against. What you are suggesting it's like virtually assigning
> tag to very each commit and tell maintainer to cope with this chaos when
> one does something in one "tag" out of 100500 ones and another person in
> another "tag". So, consider tags as stabilisation points, or points of
> stability. Then it's much easier to stick with a few tags that with 100500
> commits.

This isn't the linux kernel where there's constant churn. Most of the
time, it doesn't matter at all. Sometimes it does. Then it's a question
of if the custodian wants to do the rebase work themselves, or ask the
submitter to. And since again unlike the kernel almost no one has the
primary job of "work on U-Boot", the threshold for fixup or ask for a
rebase is much lower.

-- 
Tom

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]

Re: [PATCH v1 1/1] efi_loader: Mark a function static

[Andy Shevchenko]

On Thu, Oct 24, 2024 at 08:18:40AM -0600, Tom Rini wrote:
> On Thu, Oct 24, 2024 at 09:26:14AM +0300, Andy Shevchenko wrote:
> > On Thu, Oct 24, 2024 at 07:03:24AM +0200, Heinrich Schuchardt wrote:
> > > Am 22. Oktober 2024 15:18:45 MESZ schrieb Andy Shevchenko <andriy.shevchenko@linux.intel.com>:
> > > >On Tue, Oct 22, 2024 at 08:02:46AM +0200, Heinrich Schuchardt wrote:
> > > >> On 10/21/24 16:40, Ilias Apalodimas wrote:
> > > >> > On Mon, 21 Oct 2024 at 17:06, Andy Shevchenko
> > > >> > <andriy.shevchenko@linux.intel.com> wrote:
> > > >> > > 
> > > >> > > efi_bootmgr_release_uridp_resource() is not used anywhere except
> > > >> > > the same file where it is defined. Mark it static.
> > > >> > > This helps avoiding the compiler warning:
> > > >> > > 
> > > >> > >    lib/efi_loader/efi_bootmgr.c:388:14: warning: no previous prototype for ‘efi_bootmgr_release_uridp_resource’ [-Wmissing-prototypes]
> > > >
> > > >> The function is called efi_bootmgr_release_uridp() since 292a4a4c7b77
> > > >> ("efi_loader: shorten efi_bootmgr_release_uridp_resource()").
> > > >
> > > >Thanks! The problem is that U-Boot doesn't have the latest tag (yet)
> > > >that includes this change. You can help with managing the conflict.
> > > 
> > > Patches should be based on origin/master (or origin/next once that branch is
> > > opened typically after -rc2) and not on tags.
> > 
> > I disagree. The problem with moving target that it's been moving...
> > 
> > The tags are very good to follow and easy to maintain and test and report
> > regressions against. What you are suggesting it's like virtually assigning
> > tag to very each commit and tell maintainer to cope with this chaos when
> > one does something in one "tag" out of 100500 ones and another person in
> > another "tag". So, consider tags as stabilisation points, or points of
> > stability. Then it's much easier to stick with a few tags that with 100500
> > commits.
> 
> This isn't the linux kernel where there's constant churn. Most of the
> time, it doesn't matter at all. Sometimes it does. Then it's a question
> of if the custodian wants to do the rebase work themselves, or ask the
> submitter to. And since again unlike the kernel almost no one has the
> primary job of "work on U-Boot", the threshold for fixup or ask for a
> rebase is much lower.

I see your point, thanks for elaboration. Although I think even taking it
into account the Linux kernel approach makes some process, which is stable
(more or less) and understandable by many. I feel uncomfortable to rebase
on top of random commit. I also have scripts to update my branches and
the proposed approach breaks this badly. So I prefer to stick with my flow.
That said, consider the patch as reported problem by me. I will help testing
any new tag that will have a respective fix, or fix separately based on the tag,
so I won't need a special commits on top of.

-- 
With Best Regards,
Andy Shevchenko