[Bug 1264546] Review Request: soletta - A framework for making IoT devices

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

 



https://bugzilla.redhat.com/show_bug.cgi?id=1264546



--- Comment #35 from Paulo Andrade <paulo.cesar.pereira.de.andrade@xxxxxxxxx> ---
Hi Gustavo,

---%<---
First we will need to sort out some issues with the third party
software.

I see there is a review request for tinycbor at
https://bugzilla.redhat.com/show_bug.cgi?id=1269001
It is the same thing right?

About duktape I do not see any review or existing package.

Could tinycbor and/or duktape be used as external dependencies?
If not, what is the reason?

Note that Fedora is in the process of having more relaxed rules
for bundled packages, but it should only be used if there is no
other option.


---%<---
Another issue I noticed, is that %install appears to redo all
compilations, after regenerating header files. Some issue with
make rules?


---%<---
There are some duplicated files. I think you misunderstood some
details of adding a trailing "/". If listing only the directory,
must do it as:

%dir %{_libdir}/soletta/modules/

otherwise, there will be duplicates, because when listing a
directory it understands the directory and its contents, for
example:

%{_libdir}/soletta/modules/
%{_libdir}/soletta/modules/flow/

the line %{_libdir}/soletta/modules/ already includes
%{_libdir}/soletta/modules/flow/

Another issue is the line:

%{_includedir}/soletta/*

it should have been:

%{_includedir}/soletta/

otherwise, the directory %{_includedir}/soletta/ will not
have a owner package.


---8<---
Please comment about the warning:

In function '__fread_alias',
    inlined from 'sol_memmap_write_raw_do.isra.2' at
./src/lib/io/sol-memmap-storage.c:169:15:
/usr/include/bits/stdio2.h:290:9: warning: call to '__fread_chk_warn' declared
with attribute warning: fread called with bigger size * nmemb than length of
destination buffer
  return __fread_chk (__ptr, __bos0 (__ptr), __size, __n, __stream);

It is caused by the fread in this code:
    if (mask) {
        uint64_t value = 0, old_value;
        uint32_t i, j;

        for (i = 0, j = 0; i < entry->size; i++, j += 8)
            value |= (uint64_t)((uint8_t *)buffer->data)[i] << j;

        ret = fread(&old_value, entry->size, 1, file);

It looks suspicious. Shouldn't old_value be declared
as size_t, and instead of entry->size, write sizeof(old_value)
or sizeof(size_t) ? If the size is guaranteed to fit, it will
have issues in big endian architectures.


---8<---
Also fix the %package %{name}- issue.


---8<---
For the next review, please also update the changelog. This also
serves as a way to follow the history of the package :)

-- 
You are receiving this mail because:
You are on the CC list for the bug.
You are always notified about changes to this product and component
_______________________________________________
package-review mailing list
package-review@xxxxxxxxxxxxxxxxxxxxxxx
https://admin.fedoraproject.org/mailman/listinfo/package-review




[Index of Archives]     [Fedora Legacy]     [Fedora Desktop]     [Fedora SELinux]     [Yosemite News]     [KDE Users]     [Fedora Tools]